feat(sdk): implement zoom sdk integration#8
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis pull request introduces the MeetMind SDK v0.1.0 with complete Zoom RTMS integration. It adds environment-based configuration, SQLAlchemy persistence models for sessions and transcripts, a public Python SDK service class, Zoom webhook security validation, real-time meeting session management with callback wiring, and FastAPI HTTP endpoints for session lifecycle and transcript access. ChangesMeetMind SDK and Zoom RTMS Integration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/sdk/test_wake_words.py (1)
11-16: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd a regression test for blank/whitespace wake words.
Current tests don’t protect against false matches when configured phrases contain empty values. Add a case like
["", " ", "Atlas"]and assert only"Atlas"can trigger.🤖 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 `@tests/sdk/test_wake_words.py` around lines 11 - 16, Add a regression test to tests/sdk/test_wake_words.py that verifies detect_wake_word ignores empty or whitespace-only configured phrases: update the test_detect_wake_word_respects_configured_phrases case to include wake_words = ["", " ", "Atlas"] and assert detect_wake_word("Can you help here, Hey Atlas?", wake_words) == "Atlas" while detect_wake_word("MeetMind should not trigger", wake_words) is None (and optionally assert that inputs matching only whitespace entries do not trigger).
🤖 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 @.env.example:
- Line 41: Wrap the ZOOM_DEFAULT_WAKE_WORDS value in quotes in the .env.example
to avoid ambiguous parsing of comma-separated values with spaces (e.g., change
ZOOM_DEFAULT_WAKE_WORDS=MeetMind,Hey MeetMind to use quotes around the whole
value); update the ZOOM_DEFAULT_WAKE_WORDS entry so dotenv parsers treat the
comma and internal space as part of the value rather than tokenizing it.
In `@app/api/v1/routes/zoom_sdk.py`:
- Around line 16-31: The async endpoint zoom_rtms_webhook currently calls the
synchronous handle_zoom_webhook directly and will block the event loop; update
zoom_rtms_webhook to run the blocking handler in a worker thread by importing
and using run_in_threadpool (or asyncio.to_thread) and awaiting
run_in_threadpool(handle_zoom_webhook, db, payload) instead of calling
handle_zoom_webhook(db=db, payload=payload); keep signature and verification
logic as-is and ensure the call passes the same db and payload parameters to
handle_zoom_webhook so database operations (record_provider_event,
create_session, update_session_status) execute off the event loop.
In `@docs/sdk/zoom-rtms-sdk-prototype.md`:
- Around line 44-60: Fix the ordered-list nesting in the "Prototype Flow"
section by indenting the HTTP request code block so it is a child of the "1.
Create an SDK session:" list item (or alternatively restart numbering at 1 for
the code block), i.e., ensure the triple-backtick fenced block and its contents
are prefixed with three spaces (or otherwise indented) under the "Create an SDK
session" line so the code block is part of that list item and avoids MD029 lint
errors.
In `@sdk/config.py`:
- Line 45: The code hardcodes a single default port (5432) for DB ports (e.g.,
self.sdk_db_port), causing MySQL to fall back to the wrong port; change the
logic to pick a DB-type-specific default by reading the DB type env var (e.g.,
SDK_DB_TYPE) and mapping common types to their default ports (e.g.,
postgres/postgresql -> 5432, mysql/mariadb -> 3306, mssql -> 1433) before
calling config(..., default=...); apply the same conditional/default mapping to
the other DB port assignments in this file so each port attribute uses the
correct default for the configured DB type.
In `@sdk/models.py`:
- Line 33: The failing Ruff line-lengths are caused by overly long single-line
declarations (e.g., the field declaration "created_at: Mapped[datetime] =
mapped_column(DateTime(timezone=True), default=utc_now") and two other long
lines at positions reported (around lines 76 and 107); fix by wrapping long
arguments across multiple lines or extracting long expressions to short-named
temporaries so each source line is ≤88 chars—for example, split mapped_column
parameters onto separate indented lines or assign the DateTime(...) or default
value to a short variable and reference that variable in the field definition
(apply the same wrapping/extraction to the symbols at the other reported
locations).
- Line 65: SDKTranscriptTurn.session_id and SDKProviderEvent.session_id are
missing foreign key constraints; import ForeignKey from sqlalchemy and change
both mapped_column declarations to include ForeignKey("sdk_sessions.id") (e.g.,
session_id: Mapped[str] = mapped_column(ForeignKey("sdk_sessions.id"), String,
index=True, nullable=False)) so the columns reference sdk_sessions.id and
enforce referential integrity.
In `@sdk/providers/zoom_rtms/events.py`:
- Around line 1-3: The import block in sdk/providers/zoom_rtms/events.py is
misformatted for ruff (I001); ensure the "from __future__ import annotations"
and "from typing import Any" follow ruff's import ordering by separating the
future import from other imports with a blank line (or simply run `ruff check
--fix` in this file) so the __future__ import sits alone and the typing import
is on the next block.
- Around line 43-49: The meeting-id extraction currently builds variable `value`
from `obj` but omits the `meeting_uuid` (and possibly `meetingUuid`) keys;
update the `value = (...)` expression in the events extraction code to also
check for `obj.get("meeting_uuid")` (and `obj.get("meetingUuid")` if camelCase
variants are expected) alongside the existing keys (`meeting_id`, `meetingId`,
`meeting_number`, `meetingNumber`, `uuid`) so payloads using `meeting_uuid`
correctly resolve to a meeting id/uuid.
In `@sdk/providers/zoom_rtms/manager.py`:
- Line 35: Several raise statements and long expressions in
sdk/providers/zoom_rtms/manager.py (notably uses of ZoomRTMSRuntimeError and
long function calls/strings) exceed the 88-char Ruff E501 limit; wrap these by
breaking long strings or calls across lines using parentheses, implicit
concatenation, or by assigning parts to intermediate variables and then raising
ZoomRTMSRuntimeError(msg). Update each offending expression (the raise lines and
the other long expressions referenced) to use line breaks so no logical line
exceeds 88 chars while preserving the exact message and exception type.
- Around line 27-29: Make client lifecycle idempotent and thread-safe: add a
lock (e.g., self.clients_lock = threading.Lock()) in __init__ alongside
self.clients and self.executor, then wrap all reads/writes to self.clients
(including the handlers that process "started" and "stopped" events) with the
lock; on a "started" event check under the lock whether a live client for that
stream_id already exists and reuse it, otherwise create and register a new
client, and on a "stopped"/cleanup path ensure you close/shutdown the client and
remove it from self.clients while holding the lock so repeated events don’t leak
or duplicate clients.
- Around line 49-76: The call to client.join(...) can raise and currently leaves
the persisted session marked "listening" and the runtime client not tracked;
wrap client.join(...) in a try/except to catch join failures, and in the except
block: remove any partial runtime state (do not add self.clients[stream_id] or
pop it if added), attempt to shut down the created client (e.g., call
client.close() / client.destroy() if available on rtms.Client), and call
repo.update_session_status(session, "<rollback_status>") to revert the session
(use a stable status such as "idle" or "failed"); finally re-raise a
ZoomRTMSRuntimeError (or the original exception) so callers see the failure.
Ensure these changes are applied around the client.join(...) call and touch
symbols: client.join, self.clients, client (rtms.Client), and
repo.update_session_status.
In `@sdk/providers/zoom_rtms/webhook_security.py`:
- Around line 25-27: The current logic returns True when secret_token is missing
(checking secret_token) which effectively disables signature verification;
change this to fail-closed by returning False (or raising an error) when
secret_token is falsy, and ensure the subsequent check for timestamp and
signature also returns False if either is missing; update the function that uses
secret_token/timestamp/signature (referencing the secret_token, timestamp,
signature checks in webhook_security.py) so missing secrets or missing
timestamp/signature cause rejection rather than allowing the webhook.
In `@sdk/providers/zoom_rtms/webhook.py`:
- Around line 27-29: The URL validation branch currently casts a missing
plainToken to "None" and the line is too long; in the handler where you check if
raw_event == URL_VALIDATION_EVENT (use payload_object and URL_VALIDATION_EVENT),
extract the token into a local variable (e.g., plain_token =
payload_object(payload).get("plainToken") or
payload_object(payload).get("plain_token")), then explicitly guard that
plain_token is present and handle the invalid payload (return an appropriate
error/raise or log and return a bad-request response) instead of casting to str;
finally call zoom_url_validation_response(plain_token,
settings.zoom_webhook_secret_token) and reflow the statements so each line stays
within style limits (avoiding an inline cast and long expressions).
In `@sdk/repositories.py`:
- Around line 82-83: The current pattern of calling next_sequence(session.id)
separately then creating SDKTranscriptTurn risks duplicate sequence_no under
concurrent inserts; change sequence allocation to an atomic, transactional
operation: implement next_sequence to allocate the next sequence number inside
the same DB transaction that inserts the SDKTranscriptTurn (e.g., SELECT
MAX(sequence_no) FROM transcripts WHERE session_id = :id FOR UPDATE then use
max+1, or maintain a per-session sequence table and use UPDATE ... RETURNING or
a DB sequence), or alternatively add a unique (session_id, sequence_no)
constraint and perform the insert in a retry loop catching IntegrityError and
re-reading the next sequence; update the code paths that call next_sequence (the
caller that constructs SDKTranscriptTurn at the noted spots) to use the new
transactional allocation API so sequence assignment is atomic.
- Around line 27-29: The current expression uses "wake_words or
get_sdk_settings().zoom_default_wake_words" which treats an explicit empty list
as falsy and falls back to defaults; change the logic so configured_wake_words
is set from wake_words when the caller provided a value (including empty list)
and only use get_sdk_settings().zoom_default_wake_words when wake_words is None
(e.g., use a conditional like "wake_words if wake_words is not None else
get_sdk_settings().zoom_default_wake_words") before passing into
normalize_wake_words; update the assignment around configured_wake_words,
normalize_wake_words, wake_words, and get_sdk_settings()/zoom_default_wake_words
accordingly.
- Around line 49-55: find_zoom_session can raise MultipleResultsFound because
SDKSession.meeting_id isn't unique; modify the query in find_zoom_session to
restrict to a single row—either append .limit(1) to the select(...) before
executing or switch to using .scalars().first() on the result—so the latest
SDKSession (ordered by SDKSession.created_at.desc()) is returned safely when
multiple records exist.
- Around line 120-131: The SDKProviderEvent creation currently commits blindly
and raises IntegrityError on duplicate (provider, event_id); wrap the DB
add/commit/refresh in a try/except that catches sqlalchemy.exc.IntegrityError,
calls self.db.rollback(), then query and return the existing SDKProviderEvent by
provider and event_id (e.g., via
self.db.query(SDKProviderEvent).filter_by(provider=provider,
event_id=event_id).one()), and re-raise any other exceptions; ensure you import
IntegrityError and keep the original successful path unchanged (create event,
add, commit, refresh, return).
In `@sdk/schemas.py`:
- Line 16: Add a Pydantic field validator for SpeakRequest.text (similar to
ResetPasswordRequest.token) that strips whitespace and raises ValueError if the
result is empty so whitespace-only strings are rejected; update imports to
include pydantic.validator if missing and implement a method named e.g.
validate_text(cls, v) on the SpeakRequest model that returns the original value
when non-empty after strip and raises a ValueError with an explanatory message
otherwise.
In `@sdk/wake_words.py`:
- Around line 22-25: The detect_wake_word loop currently builds regexes from
wake_words that may be empty or whitespace; filter and strip entries before
matching so blank strings don't become boundary-only patterns. In
detect_wake_word, sanitize the input list by trimming each wake_word (e.g.,
wake_word = wake_word.strip()) and skip any empty results (or pre-filter the
wake_words iterable) before creating pattern =
rf"\b{re.escape(wake_word.lower())}\b" and running re.search; preserve the
existing sort-by-length behavior on the sanitized list.
In `@tests/sdk/test_zoom_webhook.py`:
- Around line 28-60: Add two tests: one to ensure verify_zoom_signature rejects
when secret_token is empty by calling verify_zoom_signature with secret_token=""
and asserting it returns False (or raises the expected rejection behavior), and
another to assert that meeting_id(payload) falls back to the meeting_uuid when
the payload only contains "meeting_uuid" (create a payload like
{"event":"meeting.rtms.started","payload":{"meeting_uuid":"uuid-only"}} and
assert normalize_event_name(...) == "meeting.rtms_started" and
meeting_id(payload) == "uuid-only" while rtms_stream_id(payload) remains handled
as before); reference the existing test functions verify_zoom_signature,
normalize_event_name, meeting_id, and rtms_stream_id when adding these cases.
---
Outside diff comments:
In `@tests/sdk/test_wake_words.py`:
- Around line 11-16: Add a regression test to tests/sdk/test_wake_words.py that
verifies detect_wake_word ignores empty or whitespace-only configured phrases:
update the test_detect_wake_word_respects_configured_phrases case to include
wake_words = ["", " ", "Atlas"] and assert detect_wake_word("Can you help here,
Hey Atlas?", wake_words) == "Atlas" while detect_wake_word("MeetMind should not
trigger", wake_words) is None (and optionally assert that inputs matching only
whitespace entries do not trigger).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fd24bcf-995c-418c-a7e0-9d4ec5a8120d
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.env.example.gitignoreapp/api/v1/router.pyapp/api/v1/routes/sdk.pyapp/api/v1/routes/zoom_sdk.pydocs/sdk/zoom-rtms-sdk-prototype.mdpyproject.tomlsdk/__init__.pysdk/config.pysdk/db.pysdk/models.pysdk/providers/__init__.pysdk/providers/zoom_meeting_bridge/__init__.pysdk/providers/zoom_meeting_bridge/bridge.pysdk/providers/zoom_rtms/__init__.pysdk/providers/zoom_rtms/events.pysdk/providers/zoom_rtms/manager.pysdk/providers/zoom_rtms/webhook.pysdk/providers/zoom_rtms/webhook_security.pysdk/repositories.pysdk/schemas.pysdk/sdk.pysdk/wake_words.pytests/sdk/test_config.pytests/sdk/test_repository.pytests/sdk/test_wake_words.pytests/sdk/test_zoom_webhook.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds the first Zoom RTMS-backed SDK prototype for MeetMind. This introduces a separate
sdk/package that lets developers create Zoom meeting agent sessions, configure wake words, receive Zoom RTMS webhook events, persist transcript/provider data, and prepare the boundary for future Zoom speaking support.Type of Change
feat— New featurefix— Bug fixrefactor— Code refactoring (no functional change)docs— Documentation updatetest— Adding or updating testschore— Maintenance (dependencies, CI, tooling)Related Issue
Closes #
Changes Made
sdk/package with SDK config, database setup, SQLAlchemy models, repositories, wake-word detection, and developer-facing SDK service./speakboundary.SDK_DATABASE_URL, SQLite fallback, or Postgres/MySQL env vars.ZOOM_DEFAULT_WAKE_WORDSand per-session overrides.rtmsandpython-decoupledependencies and ignored local SDK SQLite folders/files.Proof of Work
API Response / Screenshots
Test Cases
Test output
uv run pytest tests/sdk -q============================= test session starts =============================
platform win32 -- Python 3.13.0, pytest-9.0.3, pluggy-1.6.0
rootdir: C:\Users\WISDOM\Documents\Python Codes\HNG\MeetMind\meetmind-api
configfile: pyproject.toml
plugins: anyio-4.13.0, asyncio-1.3.0
asyncio: mode=Mode.AUTO, debug=False, asyncio_default_fixture_loop_scope=None, asyncio_default_test_loop_scope=function
collected 10 items
tests\sdk\test_config.py .... [ 40%]
tests\sdk\test_repository.py . [ 50%]
tests\sdk\test_wake_words.py .. [ 70%]
tests\sdk\test_zoom_webhook.py ... [100%]
============================= 10 passed in 8.37s ==============================
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests