ECHO-636 Rate limiting fix, webhooks, stream status, analytics #407
Conversation
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
…636-rate-limiting
…acking, webhooks Key changes: - Stream status: inline display under "Thinking..." (20s threshold) - Suggestions: switched to TEXT_FAST with structured outputs - Copy guide: "context limit" → "selection too large" - Translations: updated for all 6 languages - Analytics: tracking for overview mode, dynamic templates, announcements, retranscribe, clone/move/delete project, delete conversation, download audio, regenerate summary, edit/update report - Webhooks: conversation-level notifications (ENABLE_WEBHOOKS flag) - Fixed: report_utils.py undefined variable New files: - AGENTS.md - AI coding assistant context & deployment docs - frontend/COPY_GUIDE.md - UI copy style guide
Contributor
WalkthroughIntroduces comprehensive webhook infrastructure with CRUD operations and event dispatching, centralizes LLM invocation through a distributed router with failover support, adds real-time stream status notifications for high-load responses, expands analytics instrumentation across frontend components, and complements with database schema updates, conversation reconciliation workflows, and localization support. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
LGTM at a glance. This is a solid, well-scoped feature addition — webhooks are properly layered with dedicated service, API routes, and frontend UI; the LLM router refactor is a clean infrastructure upgrade with thoughtful lazy initialization and failover semantics; stream status notifications handle the UX nicely for high-load scenarios. The webhook collection is properly coordinated with tight task scheduling and reconciliation patterns. Localization coverage is thorough across six languages. A few things to verify during review: webhook dispatch error resilience (especially in task_dispatch_webhook retry logic), router availability checks at startup, and that the stream_with_status async generator cleanup doesn't leak tasks. The LLM call site migrations look consistent and mechanical across all modules — could spot-check one or two for kwargs handling. Overall solid engineering. 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
spashii
changed the title
spashii
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
echo/directus/sync/collections/operations.json (1)
1-388: LGTM on UUID rotations, but duplicates need cleanup.UUID updates to
resolve,reject, and_syncIdpreserve flow graph linkage with no logic changes—solid.However, confirmed 10 duplicate operation entries across both flows (2x "dutch", "filter_emails", "report_not_published", "trigger_email_flow" in ec4e7ea5... flow; 2x "send_email_english", "send_email_dutch", "failed", "email_send_operation_failed_dutch", "email_send_operation_failed" in 17703446... flow). Each duplicate pair has identical properties except different
_syncIdvalues.This looks like a sync conflict merge where both versions got retained. While the different
_syncIdvalues suggest Directus sync metadata, having duplicate operations with identical keys in the same flow creates ambiguity. Clean these up—keep one entry per operation and consolidate the_syncIdif multi-environment sync tracking is needed.echo/server/dembrane/transcribe.py (1)
171-191: Consider:_get_audio_file_objectreads full file into memoryThis helper reads entire audio files into memory and base64 encodes them. For large files this could be chunky. Not a new issue with this PR, but worth noting since it's in the hot path for the router-based correction workflow.
Per coding guidelines, this is a blocking I/O call (
file_service.get_stream) - if this function is ever called from async context, wrap withrun_in_thread_pool. Currently looks safe since it's called from sync Dramatiq tasks.echo/server/dembrane/report_utils.py (1)
261-266: Wraptoken_counterinrun_in_thread_pool.Both
token_countercalls at lines 261–266 and 325–330 are synchronous blocking operations inside the asyncget_report_content_for_projectfunction. Per guidelines, wrap CPU-heavy sync utilities to avoid event-loop stalls.Suggested fix
- summary_tokens = token_counter( - messages=[{"role": "user", "content": summary}], - model=get_completion_kwargs(REPORT_LLM)["model"], - ) + summary_tokens = await run_in_thread_pool( + token_counter, + messages=[{"role": "user", "content": summary}], + model=get_completion_kwargs(REPORT_LLM)["model"], + )- transcript_tokens = token_counter( - messages=[{"role": "user", "content": transcript}], - model=get_completion_kwargs(REPORT_LLM)["model"], - ) + transcript_tokens = await run_in_thread_pool( + token_counter, + messages=[{"role": "user", "content": transcript}], + model=get_completion_kwargs(REPORT_LLM)["model"], + )echo/server/dembrane/chat_utils.py (1)
250-254: Wraptoken_countercalls inrun_in_thread_poolfor async safety.Both calls (lines 251 and 604) execute in async functions and block the event loop. Since
litellm.token_counteris synchronous with no async variant, they must be offloaded per coding guidelines.Suggested fix
- summary_tokens = token_counter( - messages=[{"role": "user", "content": content}], - model=get_completion_kwargs(CHAT_LLM)["model"], - ) + summary_tokens = await run_in_thread_pool( + token_counter, + messages=[{"role": "user", "content": content}], + model=get_completion_kwargs(CHAT_LLM)["model"], + )- prompt_tokens = token_counter( - messages=[{"role": "user", "content": prompt}], - model=get_completion_kwargs(CHAT_LLM)["model"], - ) + prompt_tokens = await run_in_thread_pool( + token_counter, + messages=[{"role": "user", "content": prompt}], + model=get_completion_kwargs(CHAT_LLM)["model"], + )echo/frontend/src/locales/nl-NL.po (1)
214-310: Fill in missing Dutch translations for webhook copy.Several new webhook entries here still have empty
msgstrvalues, which leaves NL strings untranslated/blank in the UI. Please provide Dutch translations for all new webhook keys before release.Also applies to: 656-662, 1512-1553
🤖 Fix all issues with AI agents
In `@echo/.devcontainer/setup.sh`:
- Around line 142-151: The install_claude_code function is missing the
prerequisite apt package installation; update install_claude_code to call
ensure_apt_packages for curl and ca-certificates (like install_uv does) before
running the curl | bash step so the system has curl and CA certs available; keep
the existing command_exists check and logging around the install, but insert the
ensure_apt_packages call immediately before the curl invocation in
install_claude_code.
In `@echo/AGENTS.md`:
- Around line 11-66: The markdown has MD040/MD031/MD022 violations due to
missing blank lines before fenced code blocks and some headings; add a single
blank line before each fenced block and ensure a blank line after the preceding
paragraph for the sections named "Quick reference:", "**Frontend**
(`frontend/src/config.ts`):", "**Backend** (`server/dembrane/settings.py`):" and
the headings "### Recent Changes (testing branch)" and "### Tech Debt / Known
Issues" in echo/AGENTS.md so each fenced block and heading is separated by a
blank line to satisfy markdownlint.
In `@echo/directus/sync/collections/permissions.json`:
- Around line 1767-1786: The read permission for collection "project_webhook"
currently uses fields: ["*"] which exposes the webhook secret; update the
permission entry for action "read" on collection "project_webhook" to use an
explicit allowlist of safe fields (e.g., id, url, created_on, etc.) that omits
the secret field, or alternatively make the secret field write-only via a
dedicated rotation flow and remove it from all read permissions; locate the
permissions object with key "project_webhook" and modify the "fields" array
accordingly so secret is never returned to clients.
In `@echo/directus/sync/snapshot/fields/project_webhook/project_id.json`:
- Around line 5-48: The project_webhook.project_id field is currently nullable;
make it required to prevent orphaned webhooks by setting meta.required to true
and schema.is_nullable to false for the "project_id" field in the
project_webhook snapshot (ensure the select-dropdown-m2o interface and foreign
key to "project.id" remain unchanged); also add/update any validation rules or
validation_message for project_id if present to reflect the required constraint
so DB/schema sync and API validation enforce non-null values.
In `@echo/directus/sync/snapshot/fields/project_webhook/secret.json`:
- Around line 5-24: The "secret" field in the project_webhook collection is
currently visible and searchable; make it write-only by updating its meta: set
"hidden" to true and "searchable" to false for the field named "secret" in the
"project_webhook" collection so the value cannot be read or returned via
Directus UI/API and cannot be used in searches. Locate the "meta" block for
field "secret" in the snapshot and change those two keys accordingly.
In `@echo/directus/sync/snapshot/fields/project_webhook/url.json`:
- Around line 1-43: The project_webhook.url field currently allows nulls and is
optional; make it required and non-nullable by setting meta.required to true and
schema.is_nullable to false for the "url" field in the snapshot JSON (field:
"url", collection: "project_webhook"); also review schema.default_value (remove
or set to a non-null default if appropriate) and ensure any validation or
interface options reflect the required constraint so migration/import will
enforce a non-empty URL.
In
`@echo/directus/sync/snapshot/fields/project/default_conversation_tutorial_slug.json`:
- Line 19: The dropdown choice value ("value": "None") and the
schema.default_value ("none") are mismatched causing case-sensitive mismatch;
update them to match by either changing the choice "value" to "none" or changing
schema.default_value to "None" so both use the same exact string, and make the
same change for the second occurrence of the choice elsewhere in this JSON (the
other entry that currently shows the same mismatch).
- Line 14: In the JSON file default_conversation_tutorial_slug.json update the
"note" field value to fix the spelling: change "Manually syncronize this with
https://admin-dembrane.azurewebsites.net/admin/content/echo__portal_tutorial" to
use "synchronize" instead of "syncronize" so the "note" string is spelled
correctly.
In `@echo/frontend/COPY_GUIDE.md`:
- Around line 64-66: Update the copy in the "### Rules" section: change the
phrase "Always keep text left aligned (unless very short and wrapped in a
border)" to hyphenate the compound modifier as "Always keep text left-aligned
(unless very short and wrapped in a border)"; edit the rule text in
COPY_GUIDE.md so the term "left-aligned" uses a hyphen for correct
compound-modifier style.
In
`@echo/frontend/src/components/participant/ParticipantConversationAudioContent.tsx`:
- Around line 71-77: useLoadNotification is being called but its returned values
(e.g., isHighLoad, statusMessage) are ignored so it does nothing; update the
call in ParticipantConversationAudioContent to destructure the hook result
(e.g., const { isHighLoad, statusMessage } = useLoadNotification({ data:
echoData, isLoading: echoIsLoading, hasContent: hasEchoContent })) and then
either: (a) trigger the notification/toast when isHighLoad becomes true or
render statusMessage in the component UI, or (b) if you don't need the status,
remove the useLoadNotification invocation and related variables (hasEchoContent,
echoData/echoIsLoading usage) to eliminate dead code; reference
useLoadNotification, hasEchoContent, echoData, and echoIsLoading to locate and
fix the integration.
In `@echo/frontend/src/components/project/webhooks/WebhookSettingsCard.tsx`:
- Around line 647-656: In WebhookSettingsCard, update the external Anchor
elements (the Anchor with href "https://www.make.com/en/blog/what-are-webhooks"
and the other Anchor block around lines 830-839) to include rel="noopener
noreferrer" alongside target="_blank" to prevent reverse-tabnabbing; locate the
Anchor usages inside the WebhookSettingsCard component and add the rel prop to
each external link.
- Around line 46-65: Update the static strings in WEBHOOK_EVENTS and the
fallback name "Unnamed Webhook" to use Lingui's t`...` so they are included in
localization extraction; specifically wrap each label and description in the
WEBHOOK_EVENTS array with t`...` and replace the literal fallback string used
for unnamed webhooks (search for the fallback usage/constant or UI text) with
t`...` as well so both the array entries and the fallback are picked up by
message extraction. Ensure you import t from `@lingui/macro` if not already
imported and keep the WebhookEvent values unchanged.
- Around line 121-149: The form currently swallows mutation errors and always
resets/closes on submit; modify WebhookSettingsCard so onSubmit only calls
reset() and onClose() after a successful mutation (use the promise return of
updateMutation.mutateAsync/createMutation.mutateAsync) and do not close on
error, then surface mutation errors inside the modal by rendering a Mantine
Alert in the modal Stack: read error state from updateMutation.error and
createMutation.error (or their isError/message fields) and show a contextual
Alert above the form inputs when present; keep existing toast behavior but pair
it with this inline Alert so users see errors without the modal closing.
- Around line 365-367: The useEffect currently only runs on mount and resets
optimisticEnabled once; change its dependency array to include the webhook (or
webhook.status) prop so it also runs whenever the webhook is updated from server
refetches. Update the effect that calls setOptimisticEnabled(null) to depend on
webhook or webhook.status (e.g., useEffect(..., [webhook, webhook?.status])) so
the toggle state is cleared/reset whenever the server-provided webhook changes.
In `@echo/frontend/src/hooks/useLoadNotification.ts`:
- Around line 78-95: statusEvents is recomputed on every render because
parseStatusEvents(data) returns a new array reference, causing the useEffect
(which watches statusEvents) to run unnecessarily; wrap the
parseStatusEvents(data) call in useMemo (or otherwise memoize its result) so
statusEvents only changes when data changes (referencing parseStatusEvents and
statusEvents), or alternatively move parsing into the effect and depend on data
directly (the useEffect that uses prevDataLengthRef, latestEvent, setIsHighLoad,
setStatusMessage should then depend on the memoized statusEvents or on data if
you choose the latter) to avoid needless effect runs.
In `@echo/frontend/src/locales/de-DE.po`:
- Around line 205-220: The de-DE locale file is missing translations for new
webhook UI strings used in WebhookSettingsCard (e.g., the msgids "1. You provide
a URL where you want to receive notifications", "2. When a conversation event
happens, we automatically send the conversation data to your URL", "3. Your
system receives the data and can act on it (e.g., save to a database, send an
email, update a spreadsheet)" and "A friendly name to identify this webhook");
update the corresponding msgstr entries with German translations (or temporary
English copies) so labels/prompts render for German users and repeat the same
for the other listed msgids referenced in the review (all occurrences noted in
the comment). Ensure each msgid in echo/frontend/src/locales/de-DE.po has a
non-empty msgstr matching the UI text from WebhookSettingsCard.tsx and related
components.
In `@echo/frontend/src/locales/en-US.po`:
- Around line 1166-1169: The PO file contains an obsolete translation entry for
msgid "Context limit reached" (marked with `#~`) originating from
SelectAllConfirmationModal.tsx; remove that obsolete block (the lines beginning
with `#~ msgid "Context limit reached"` and `#~ msgstr "Context limit reached"`)
to prune dead strings, or if you need to keep it for reference, add a short
comment and ensure the active string "Selection too large" is the canonical
msgid used by SelectAllConfirmationModal.tsx.
In `@echo/frontend/src/locales/es-ES.po`:
- Around line 205-220: The es-ES .po entries for the WebhookSettingsCard strings
(msgids: "1. You provide a URL where you want to receive notifications", "2.
When a conversation event happens, we automatically send the conversation data
to your URL", "3. Your system receives the data and can act on it (e.g., save to
a database, send an email, update a spreadsheet)", and "A friendly name to
identify this webhook") are empty; do not manually edit the .po — instead re-run
the Lingui extraction/translation workflow (i18n tooling) to regenerate/update
translations and then provide Spanish msgstr values for those msgids so es-ES is
populated before release; verify the strings are emitted from
WebhookSettingsCard and committed as part of the translation bundle.
In `@echo/frontend/src/locales/fr-FR.po`:
- Around line 205-219: The fr-FR.po entries for the Webhook UI (msgids such as
"1. You provide a URL where you want to receive notifications", "2. When a
conversation event happens, we automatically send the conversation data to your
URL", "3. Your system receives the data and can act on it (e.g., save to a
database, send an email, update a spreadsheet)" and "A friendly name to identify
this webhook" referenced from
src/components/project/webhooks/WebhookSettingsCard.tsx are missing
translations; edit echo/frontend/src/locales/fr-FR.po and provide appropriate
French strings in each corresponding msgstr (or, if unsure, mark the entry as
fuzzy) so the fr-FR locale no longer falls back to English—apply the same fix
for the other empty msgstr blocks listed in the review (all other line ranges
referenced) to ensure full French localization.
In `@echo/frontend/src/locales/it-IT.po`:
- Around line 454-469: Missing Italian translations for webhook UI strings:
populate the empty msgstr entries in it-IT.po for the webhook-related msgid
texts referenced by the WebhookSettingsCard component (e.g., "1. You provide a
URL where you want to receive notifications", "2. When a conversation event
happens, we automatically send the conversation data to your URL", "3. Your
system receives the data and can act on it (e.g., save to a database, send an
email, update a spreadsheet)", and "A friendly name to identify this webhook")
and for the other ranges noted (539-547, 1337-1388); update each empty msgstr
with an appropriate Italian translation (or explicitly copy the English string
if you intend that as the fallback) so the WebhookSettingsCard UI renders
localized text for Italian users.
In `@echo/server/AGENTS.md`:
- Around line 45-72: Static analysis reports markdownlint rule MD022 (blank
lines around headings) for the "Background Task Design Patterns" section and the
emphasized list items; fix by either adding a markdownlint ignore comment for
MD022 at the top of this AGENTS.md section or by adjusting the spacing so there
are proper blank lines around the "Background Task Design Patterns" heading and
the numbered list items (the bold state-flags like is_finished,
is_all_chunks_transcribed, summary) to satisfy the linter; apply one consistent
approach (disable MD022 for this file if CI enforces linting) and commit the
change.
In `@echo/server/dembrane/api/project_webhook.py`:
- Around line 107-128: The Directus SDK calls inside the directus_client_context
(e.g., the with directus_client_context(auth.client) as client block and
subsequent client.get_items calls in project_webhook.py) are synchronous and
must be executed via run_in_thread_pool to avoid blocking the event loop; change
each usage to call run_in_thread_pool for the blocking work (entering the
context and any client.* calls such as get_items, create_item, update_item,
delete_item) and await the result, applying the same pattern to the other listed
blocks (around lines handling project_webhook calls at the other ranges
mentioned) so all directus_client_context/client.* interactions are executed
inside run_in_thread_pool.
In `@echo/server/dembrane/conversation_utils.py`:
- Around line 120-124: The loop that appends to conversation_ids uses
conversation["id"] in both the try and the except log, which will re-raise if id
is missing; change the logic to first read cid = conversation.get("id") (or cid
= conversation.get("id", "<missing>")), only append when cid is truthy, and use
cid in the logger instead of indexing conversation; make the same change for the
similar block at lines ~164-168 so both loops use conversation.get("id") and
avoid re-raising during error logging (update references to conversation_ids and
logger in conversation_utils.py accordingly).
- Around line 91-116: The Directus synchronous call to directus.get_items in the
conversation fetch (the "conversation" query that assigns to response) must be
moved into an async wrapper that runs the blocking I/O in a thread pool (e.g.,
use asyncio.to_thread or the project's run_in_thread_pool/run_async_in_new_loop
helper); implement an async function (e.g., async_fetch_conversations or
async_get_conversation_items) that calls directus.get_items with the same
query/filter/fields/limit and returns the result, then replace direct
synchronous usage with awaiting that async wrapper at all call sites that may
run in async contexts so the event loop is never blocked. Ensure the wrapper
preserves the same parameters (limit, filters including
is_finished/is_all_chunks_transcribed/chunks logic) and update callers to await
the new async function.
In `@echo/server/dembrane/coordination.py`:
- Around line 413-416: Replace the non-atomic setnx+expire pattern with an
atomic Redis SET using NX and EX options: where the code currently calls
client.setnx(key, "1") followed by client.expire(key,
_SUMMARIZE_LOCK_TTL_SECONDS), change it to call the single atomic operation
(e.g., client.set(key, "1", nx=True, ex=_SUMMARIZE_LOCK_TTL_SECONDS)) inside the
same try block so the lock is created with TTL in one round-trip and avoid
orphan keys if the process crashes.
In `@echo/server/dembrane/llm_router.py`:
- Around line 39-41: Concurrent calls can race and build the global _router more
than once; add a module-level lock (e.g., _router_lock = threading.Lock()) and
use a double-checked locking pattern around the Router creation: check if
_router is None, acquire _router_lock, check again, then create and assign the
Router. Apply the same guarded pattern to the other lazy-init site that also
touches _router (the second Router-initialization block referenced) so all lazy
inits are single-flight. Ensure you import threading and only hold the lock
during creation/assignment, not during subsequent router use.
- Around line 136-152: The _parse_redis_url helper is incorrectly stripping the
DB index which LiteLLM Router doesn't accept; instead stop parsing and pass the
original redis_url string directly into the Router (replace uses of
_parse_redis_url with the raw redis_url value when constructing the Router). If
you choose to keep _parse_redis_url for host/port/password extraction, change it
to detect a non-empty parsed.path and emit a warning/error via the logger (e.g.,
mention parsed.path contains a DB index) so misconfigured URLs are caught;
reference the _parse_redis_url function and the Router constructor where the
redis connection is created to make the change.
In `@echo/server/dembrane/service/conversation.py`:
- Around line 304-316: The webhook dispatch call to dispatch_webhooks_for_event
in the conversation creation path may perform blocking I/O and should be invoked
via run_in_thread_pool when used from async contexts: replace the direct call to
dispatch_webhooks_for_event with a call scheduled through
run_in_thread_pool(dispatch_webhooks_for_event, project_id=project_id,
conversation_id=new_conversation["id"], event="conversation.created") (or the
equivalent call-site adaptation) so it won't block the event loop; also update
the exception logger call in the except block to use lazy formatting
logger.warning("Failed to dispatch conversation.created webhook: %s", e) instead
of an f-string.
In `@echo/server/dembrane/service/webhook.py`:
- Around line 68-196: The code makes synchronous Directus calls and HTTP
requests that block the event loop; update get_webhooks_for_project,
build_transcript, dispatch_webhook_sync and enqueue_webhooks_for_event to call
blocking operations via run_in_thread_pool (or an equivalent asyncio.to_thread
wrapper) instead of calling client.get_items() and requests.post() directly:
change the Directus client get_items invocations inside get_webhooks_for_project
and build_transcript to be executed through run_in_thread_pool and awaited, wrap
the requests.post call in dispatch_webhook_sync with run_in_thread_pool, and
ensure enqueue_webhooks_for_event awaits those thread-wrapped helpers (or
convert these higher-level functions to async and await the thread-wrapped
calls); also add the necessary import for run_in_thread_pool and propagate
async/await changes to callers.
In `@echo/server/dembrane/settings.py`:
- Around line 197-199: get_all_model_groups currently returns a hardcoded list
that can drift from the LLMProviderConfig fields; update get_all_model_groups to
derive its return values from the LLMProviderConfig definition (e.g., inspect
LLMProviderConfig.__annotations__ or dataclass/attrs fields to build the list
dynamically) or, minimally, add a clear "# NOTE: keep in sync with
LLMProviderConfig" comment above get_all_model_groups; reference the function
name get_all_model_groups and the LLMProviderConfig symbol when making the
change.
In `@echo/server/dembrane/stream_status.py`:
- Around line 20-27: StreamStatusType declares PROCESSING, RETRYING, and READY
but they aren't used; either remove those unused enum members from
StreamStatusType to eliminate dead code, or if they're intended for future use,
keep them but add an explicit comment like "reserved for future use" next to
each member (or a module-level TODO) so it's clear they're intentional; update
any tests or usages accordingly.
- Around line 86-91: Remove the redundant call to stream_generator.__aiter__()
and the unused stream_iter variable: use the AsyncGenerator object
stream_generator directly when iterating in the reader (update the reader
coroutine that currently consumes stream_iter to iterate over stream_generator);
keep chunk_queue as-is and ensure any references to stream_iter are replaced
with stream_generator in the reader logic.
- Around line 122-123: Replace the self-chaining raises `raise error_data from
error_data` with the correct pattern: if you want to preserve the original
traceback, change to a plain `raise error_data`; if you intend to suppress
exception chaining, change to `raise error_data from None`. Update both
occurrences that use the `error_data` variable (the `raise error_data from
error_data` expressions) so they either use plain raise or `from None`
consistently depending on the intended behavior.
In `@echo/server/dembrane/tasks.py`:
- Around line 904-918: The current logic treats all 4xx codes as non-retriable
which incorrectly drops 408 and 429; update the webhook response handling in the
function that uses webhook_id, status_code, logger and raises
WebhookServiceException so that 408 (Request Timeout) and 429 (Too Many
Requests) are treated as retriable: either add an explicit branch that checks if
status_code in (408, 429) and logs a warning then raise WebhookServiceException,
or change the existing 400 <= status_code < 500 branch to exclude 408 and 429
(e.g., 400 <= status_code < 500 and status_code not in (408,429)); keep
non-retriable 4xx logging for other client errors and preserve the 2xx success
return path.
Comment on lines
+142
to
+151
| install_claude_code() { | ||
| if command_exists claude; then | ||
| log_info "claude already installed: $(claude --version)" | ||
| return | ||
| fi | ||
|
|
||
| log_info "Installing claude globally..." | ||
| curl -fsSL https://claude.ai/install.sh | bash | ||
| log_info "claude installed" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd echo && head -200 .devcontainer/setup.sh | tail -100Repository: Dembrane/echo
Length of output: 2912
🏁 Script executed:
# Search for install_codex function to compare patterns
rg -A 10 "install_codex" echo/.devcontainer/setup.shRepository: Dembrane/echo
Length of output: 591
🌐 Web query:
Claude CLI official installation method
💡 Result:
Official install (Claude Code / Claude CLI):
-
Recommended (npm): npm install -g
@anthropic-ai/claude-code(do NOT use sudo; runclaude --versionto verify). [1] -
Native installer (beta / alternative):
- macOS / Linux / WSL: curl -fsSL https://claude.ai/install.sh | bash
- Windows PowerShell: irm https://claude.ai/install.ps1 | iex. [2]
After install run claude auth login (or claude doctor) to authenticate/check the install. [1][2]
Sources:
[1] Anthropic Claude Code docs — Quickstart (npm install).
[2] Anthropic Claude Code docs — Native install instructions.
Add prerequisite system packages before the curl command.
The install URL is legit per official Anthropic docs, but you're missing the prerequisite check. Look at install_uv — it calls ensure_apt_packages curl ca-certificates before curling. Do that here too:
ensure_apt_packages curl ca-certificates
log_info "Installing claude globally..."
curl -fsSL https://claude.ai/install.sh | bash
🤖 Prompt for AI Agents
In `@echo/.devcontainer/setup.sh` around lines 142 - 151, The install_claude_code
function is missing the prerequisite apt package installation; update
install_claude_code to call ensure_apt_packages for curl and ca-certificates
(like install_uv does) before running the curl | bash step so the system has
curl and CA certs available; keep the existing command_exists check and logging
around the install, but insert the ensure_apt_packages call immediately before
the curl invocation in install_claude_code.
Comment on lines
+11
to
+66
| ``` | ||
| echo/ | ||
| ├── frontend/ # React + Vite frontend | ||
| │ ├── src/ | ||
| │ │ ├── components/ | ||
| │ │ ├── routes/ | ||
| │ │ ├── locales/ # Translation .po files | ||
| │ │ └── config.ts # Feature flags | ||
| │ └── COPY_GUIDE.md # UI copy style guide | ||
| ├── server/ # Python FastAPI backend | ||
| │ └── dembrane/ | ||
| │ ├── api/ # API endpoints | ||
| │ ├── service/ # Business logic | ||
| │ └── settings.py # Configuration & feature flags | ||
| └── docs/ # Documentation | ||
| ``` | ||
|
|
||
| ## Key Conventions | ||
|
|
||
| ### UI Copy (IMPORTANT) | ||
|
|
||
| Always follow [frontend/COPY_GUIDE.md](frontend/COPY_GUIDE.md) when writing user-facing text: | ||
|
|
||
| - **Shortest possible, highest clarity** | ||
| - **No jargon** — use plain language users understand | ||
| - **No corporate speak** — write like explaining to a colleague | ||
| - **Never say "successfully"** — just state what happened | ||
|
|
||
| Examples: | ||
| - "Context limit reached" → "Selection too large" | ||
| - "Successfully saved" → "Saved" | ||
| - "Please wait while we process" → "Processing..." | ||
|
|
||
| ### Translations | ||
|
|
||
| See [docs/frontend_translations.md](docs/frontend_translations.md) for the full workflow. | ||
|
|
||
| Quick reference: | ||
| ```bash | ||
| cd frontend | ||
| pnpm messages:extract # Extract new strings to .po files | ||
| # Edit .po files in src/locales/ | ||
| pnpm messages:compile # Compile for production | ||
| ``` | ||
|
|
||
| Supported languages: en-US, nl-NL, de-DE, fr-FR, es-ES, it-IT | ||
|
|
||
| ### Feature Flags | ||
|
|
||
| **Frontend** (`frontend/src/config.ts`): | ||
| ```typescript | ||
| export const ENABLE_FEATURE_NAME = import.meta.env.VITE_ENABLE_FEATURE_NAME === "1"; | ||
| ``` | ||
|
|
||
| **Backend** (`server/dembrane/settings.py`): | ||
| ```python |
Contributor
There was a problem hiding this comment.
Fix markdownlint violations (fences + heading spacing).
LGTM after we clean up MD040/MD031/MD022 to keep doc lint green.
📝 Minimal lint fix
-```
+```text
echo/
├── frontend/ # React + Vite frontend
@@
-Quick reference:
-```bash
+Quick reference:
+
+```bash
cd frontend
pnpm messages:extract # Extract new strings to .po files
# Edit .po files in src/locales/
pnpm messages:compile # Compile for production@@
-Frontend (frontend/src/config.ts):
-typescript +**Frontend** (`frontend/src/config.ts`): + +typescript
export const ENABLE_FEATURE_NAME = import.meta.env.VITE_ENABLE_FEATURE_NAME === "1";
-**Backend** (`server/dembrane/settings.py`):
-```python
+**Backend** (`server/dembrane/settings.py`):
+
+```python
feature_name: bool = Field(
@@
-### Recent Changes (testing branch)
+
+### Recent Changes (testing branch)
@@
-### Tech Debt / Known Issues
+
+### Tech Debt / Known Issues
Also applies to: 125-132
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
49-49: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
61-61: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
66-66: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@echo/AGENTS.md` around lines 11 - 66, The markdown has MD040/MD031/MD022
violations due to missing blank lines before fenced code blocks and some
headings; add a single blank line before each fenced block and ensure a blank
line after the preceding paragraph for the sections named "Quick reference:",
"**Frontend** (`frontend/src/config.ts`):", "**Backend**
(`server/dembrane/settings.py`):" and the headings "### Recent Changes (testing
branch)" and "### Tech Debt / Known Issues" in echo/AGENTS.md so each fenced
block and heading is separated by a blank line to satisfy markdownlint.
Comment on lines
+1767
to
+1786
| "collection": "project_webhook", | ||
| "action": "read", | ||
| "permissions": { | ||
| "_and": [ | ||
| { | ||
| "project_id": { | ||
| "directus_user_id": { | ||
| "_eq": "$CURRENT_USER" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "validation": null, | ||
| "presets": null, | ||
| "fields": [ | ||
| "*" | ||
| ], | ||
| "policy": "37a60e48-dd00-4867-af07-1fb22ac89078", | ||
| "_syncId": "7587cc08-f56d-4177-9f0a-b3708c775c50" |
Contributor
There was a problem hiding this comment.
Do not expose webhook secrets in read permissions.
Not LGTM: fields: ["*"] will return secret to clients. Switch to an explicit allowlist that omits secret (or keep secret write-only via a dedicated rotation flow).
🔐 Safer read field allowlist
- "fields": [
- "*"
- ],
+ "fields": [
+ "id",
+ "name",
+ "url",
+ "events",
+ "status",
+ "project_id",
+ "date_created",
+ "date_updated",
+ "user_created",
+ "user_updated"
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "collection": "project_webhook", | |
| "action": "read", | |
| "permissions": { | |
| "_and": [ | |
| { | |
| "project_id": { | |
| "directus_user_id": { | |
| "_eq": "$CURRENT_USER" | |
| } | |
| } | |
| } | |
| ] | |
| }, | |
| "validation": null, | |
| "presets": null, | |
| "fields": [ | |
| "*" | |
| ], | |
| "policy": "37a60e48-dd00-4867-af07-1fb22ac89078", | |
| "_syncId": "7587cc08-f56d-4177-9f0a-b3708c775c50" | |
| "collection": "project_webhook", | |
| "action": "read", | |
| "permissions": { | |
| "_and": [ | |
| { | |
| "project_id": { | |
| "directus_user_id": { | |
| "_eq": "$CURRENT_USER" | |
| } | |
| } | |
| } | |
| ] | |
| }, | |
| "validation": null, | |
| "presets": null, | |
| "fields": [ | |
| "id", | |
| "name", | |
| "url", | |
| "events", | |
| "status", | |
| "project_id", | |
| "date_created", | |
| "date_updated", | |
| "user_created", | |
| "user_updated" | |
| ], | |
| "policy": "37a60e48-dd00-4867-af07-1fb22ac89078", | |
| "_syncId": "7587cc08-f56d-4177-9f0a-b3708c775c50" |
🤖 Prompt for AI Agents
In `@echo/directus/sync/collections/permissions.json` around lines 1767 - 1786,
The read permission for collection "project_webhook" currently uses fields:
["*"] which exposes the webhook secret; update the permission entry for action
"read" on collection "project_webhook" to use an explicit allowlist of safe
fields (e.g., id, url, created_on, etc.) that omits the secret field, or
alternatively make the secret field write-only via a dedicated rotation flow and
remove it from all read permissions; locate the permissions object with key
"project_webhook" and modify the "fields" array accordingly so secret is never
returned to clients.
Comment on lines
+5
to
+48
| "meta": { | ||
| "collection": "project_webhook", | ||
| "conditions": null, | ||
| "display": null, | ||
| "display_options": null, | ||
| "field": "project_id", | ||
| "group": null, | ||
| "hidden": false, | ||
| "interface": "select-dropdown-m2o", | ||
| "note": null, | ||
| "options": { | ||
| "enableLink": true, | ||
| "template": "{{name}}" | ||
| }, | ||
| "readonly": false, | ||
| "required": false, | ||
| "searchable": true, | ||
| "sort": 11, | ||
| "special": [ | ||
| "m2o" | ||
| ], | ||
| "translations": null, | ||
| "validation": null, | ||
| "validation_message": null, | ||
| "width": "full" | ||
| }, | ||
| "schema": { | ||
| "name": "project_id", | ||
| "table": "project_webhook", | ||
| "data_type": "uuid", | ||
| "default_value": null, | ||
| "max_length": null, | ||
| "numeric_precision": null, | ||
| "numeric_scale": null, | ||
| "is_nullable": true, | ||
| "is_unique": false, | ||
| "is_indexed": false, | ||
| "is_primary_key": false, | ||
| "is_generated": false, | ||
| "generation_expression": null, | ||
| "has_auto_increment": false, | ||
| "foreign_key_table": "project", | ||
| "foreign_key_column": "id" | ||
| } |
Contributor
There was a problem hiding this comment.
Require project_id to prevent orphaned webhooks.
Not LGTM: webhooks should always belong to a project; nullable/optional invites invalid rows and permission edge cases.
✅ Enforce required project_id
- "required": false,
+ "required": true,
@@
- "is_nullable": true,
+ "is_nullable": false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "meta": { | |
| "collection": "project_webhook", | |
| "conditions": null, | |
| "display": null, | |
| "display_options": null, | |
| "field": "project_id", | |
| "group": null, | |
| "hidden": false, | |
| "interface": "select-dropdown-m2o", | |
| "note": null, | |
| "options": { | |
| "enableLink": true, | |
| "template": "{{name}}" | |
| }, | |
| "readonly": false, | |
| "required": false, | |
| "searchable": true, | |
| "sort": 11, | |
| "special": [ | |
| "m2o" | |
| ], | |
| "translations": null, | |
| "validation": null, | |
| "validation_message": null, | |
| "width": "full" | |
| }, | |
| "schema": { | |
| "name": "project_id", | |
| "table": "project_webhook", | |
| "data_type": "uuid", | |
| "default_value": null, | |
| "max_length": null, | |
| "numeric_precision": null, | |
| "numeric_scale": null, | |
| "is_nullable": true, | |
| "is_unique": false, | |
| "is_indexed": false, | |
| "is_primary_key": false, | |
| "is_generated": false, | |
| "generation_expression": null, | |
| "has_auto_increment": false, | |
| "foreign_key_table": "project", | |
| "foreign_key_column": "id" | |
| } | |
| "meta": { | |
| "collection": "project_webhook", | |
| "conditions": null, | |
| "display": null, | |
| "display_options": null, | |
| "field": "project_id", | |
| "group": null, | |
| "hidden": false, | |
| "interface": "select-dropdown-m2o", | |
| "note": null, | |
| "options": { | |
| "enableLink": true, | |
| "template": "{{name}}" | |
| }, | |
| "readonly": false, | |
| "required": true, | |
| "searchable": true, | |
| "sort": 11, | |
| "special": [ | |
| "m2o" | |
| ], | |
| "translations": null, | |
| "validation": null, | |
| "validation_message": null, | |
| "width": "full" | |
| }, | |
| "schema": { | |
| "name": "project_id", | |
| "table": "project_webhook", | |
| "data_type": "uuid", | |
| "default_value": null, | |
| "max_length": null, | |
| "numeric_precision": null, | |
| "numeric_scale": null, | |
| "is_nullable": false, | |
| "is_unique": false, | |
| "is_indexed": false, | |
| "is_primary_key": false, | |
| "is_generated": false, | |
| "generation_expression": null, | |
| "has_auto_increment": false, | |
| "foreign_key_table": "project", | |
| "foreign_key_column": "id" | |
| } |
🤖 Prompt for AI Agents
In `@echo/directus/sync/snapshot/fields/project_webhook/project_id.json` around
lines 5 - 48, The project_webhook.project_id field is currently nullable; make
it required to prevent orphaned webhooks by setting meta.required to true and
schema.is_nullable to false for the "project_id" field in the project_webhook
snapshot (ensure the select-dropdown-m2o interface and foreign key to
"project.id" remain unchanged); also add/update any validation rules or
validation_message for project_id if present to reflect the required constraint
so DB/schema sync and API validation enforce non-null values.
Comment on lines
+5
to
+24
| "meta": { | ||
| "collection": "project_webhook", | ||
| "conditions": null, | ||
| "display": null, | ||
| "display_options": null, | ||
| "field": "secret", | ||
| "group": null, | ||
| "hidden": false, | ||
| "interface": "input", | ||
| "note": null, | ||
| "options": null, | ||
| "readonly": false, | ||
| "required": false, | ||
| "searchable": true, | ||
| "sort": 9, | ||
| "special": null, | ||
| "translations": null, | ||
| "validation": null, | ||
| "validation_message": null, | ||
| "width": "full" |
Contributor
There was a problem hiding this comment.
Lock down webhook secrets (write-only + non-searchable).
Not LGTM yet: this field is visible and searchable, which risks accidental disclosure through Directus UI/API. Make it hidden and non-searchable so secrets are write-only by default.
🔒 Suggested hardening
- "hidden": false,
+ "hidden": true,
...
- "searchable": true,
+ "searchable": false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "meta": { | |
| "collection": "project_webhook", | |
| "conditions": null, | |
| "display": null, | |
| "display_options": null, | |
| "field": "secret", | |
| "group": null, | |
| "hidden": false, | |
| "interface": "input", | |
| "note": null, | |
| "options": null, | |
| "readonly": false, | |
| "required": false, | |
| "searchable": true, | |
| "sort": 9, | |
| "special": null, | |
| "translations": null, | |
| "validation": null, | |
| "validation_message": null, | |
| "width": "full" | |
| "meta": { | |
| "collection": "project_webhook", | |
| "conditions": null, | |
| "display": null, | |
| "display_options": null, | |
| "field": "secret", | |
| "group": null, | |
| "hidden": true, | |
| "interface": "input", | |
| "note": null, | |
| "options": null, | |
| "readonly": false, | |
| "required": false, | |
| "searchable": false, | |
| "sort": 9, | |
| "special": null, | |
| "translations": null, | |
| "validation": null, | |
| "validation_message": null, | |
| "width": "full" |
🤖 Prompt for AI Agents
In `@echo/directus/sync/snapshot/fields/project_webhook/secret.json` around lines
5 - 24, The "secret" field in the project_webhook collection is currently
visible and searchable; make it write-only by updating its meta: set "hidden" to
true and "searchable" to false for the field named "secret" in the
"project_webhook" collection so the value cannot be read or returned via
Directus UI/API and cannot be used in searches. Locate the "meta" block for
field "secret" in the snapshot and change those two keys accordingly.
Comment on lines
+197
to
+199
| def get_all_model_groups(self) -> List[str]: | ||
| """Return all known model group names.""" | ||
| return ["multi_modal_pro", "multi_modal_fast", "text_fast"] |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded group list - future maintenance note.
This list should stay in sync with the LLMProviderConfig fields above (lines 126-128). Consider a # NOTE: comment or deriving from field names if this grows.
🤖 Prompt for AI Agents
In `@echo/server/dembrane/settings.py` around lines 197 - 199,
get_all_model_groups currently returns a hardcoded list that can drift from the
LLMProviderConfig fields; update get_all_model_groups to derive its return
values from the LLMProviderConfig definition (e.g., inspect
LLMProviderConfig.__annotations__ or dataclass/attrs fields to build the list
dynamically) or, minimally, add a clear "# NOTE: keep in sync with
LLMProviderConfig" comment above get_all_model_groups; reference the function
name get_all_model_groups and the LLMProviderConfig symbol when making the
change.
Comment on lines
+20
to
+27
| class StreamStatusType(str, Enum): | ||
| """Status event types for stream notifications.""" | ||
|
|
||
| PROCESSING = "processing" # Initial processing started | ||
| RETRYING = "retrying" # Request is being retried | ||
| HIGH_LOAD = "high_load" # System experiencing high demand | ||
| READY = "ready" # Response starting | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused enum values — ship it or strip it.
PROCESSING, RETRYING, and READY are defined but never used in this module. If they're for future extensibility, that's cool — just be aware they're dead code until then.
🤖 Prompt for AI Agents
In `@echo/server/dembrane/stream_status.py` around lines 20 - 27, StreamStatusType
declares PROCESSING, RETRYING, and READY but they aren't used; either remove
those unused enum members from StreamStatusType to eliminate dead code, or if
they're intended for future use, keep them but add an explicit comment like
"reserved for future use" next to each member (or a module-level TODO) so it's
clear they're intentional; update any tests or usages accordingly.
Comment on lines
+86
to
+91
|
|
||
| # Create an async iterator from the generator | ||
| stream_iter = stream_generator.__aiter__() | ||
|
|
||
| # Queue to hold chunks from the stream | ||
| chunk_queue: asyncio.Queue[tuple[str | None, Exception | None]] = asyncio.Queue() |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor nits on iterator handling.
Line 88: stream_generator.__aiter__() is redundant — stream_generator is already an AsyncGenerator and is its own iterator. You can just iterate directly over stream_generator in the reader.
- # Create an async iterator from the generator
- stream_iter = stream_generator.__aiter__()
-
- # Queue to hold chunks from the stream
- chunk_queue: asyncio.Queue[tuple[str | None, Exception | None]] = asyncio.Queue()
+ # Queue to hold chunks from the stream
+ chunk_queue: asyncio.Queue[tuple[str | None, Exception | None]] = asyncio.Queue()Then update the reader:
- async for chunk in stream_iter:
+ async for chunk in stream_generator:🤖 Prompt for AI Agents
In `@echo/server/dembrane/stream_status.py` around lines 86 - 91, Remove the
redundant call to stream_generator.__aiter__() and the unused stream_iter
variable: use the AsyncGenerator object stream_generator directly when iterating
in the reader (update the reader coroutine that currently consumes stream_iter
to iterate over stream_generator); keep chunk_queue as-is and ensure any
references to stream_iter are replaced with stream_generator in the reader
logic.
Comment on lines
+122
to
+123
| if error_data is not None: | ||
| raise error_data from error_data |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Exception chaining pattern is a bit unusual.
raise error_data from error_data chains an exception to itself. If you want to preserve the original traceback, just raise error_data. If you want to suppress chaining, use from None.
- if error_data is not None:
- raise error_data from error_data
+ if error_data is not None:
+ raise error_dataSame applies to line 145.
🤖 Prompt for AI Agents
In `@echo/server/dembrane/stream_status.py` around lines 122 - 123, Replace the
self-chaining raises `raise error_data from error_data` with the correct
pattern: if you want to preserve the original traceback, change to a plain
`raise error_data`; if you intend to suppress exception chaining, change to
`raise error_data from None`. Update both occurrences that use the `error_data`
variable (the `raise error_data from error_data` expressions) so they either use
plain raise or `from None` consistently depending on the intended behavior.
Comment on lines
+904
to
+918
| # Consider 2xx as success | ||
| if 200 <= status_code < 300: | ||
| logger.info(f"Webhook {webhook_id} dispatched successfully (status: {status_code})") | ||
| return | ||
| elif 400 <= status_code < 500: | ||
| # Client errors (4xx) - don't retry, log and exit | ||
| logger.warning( | ||
| f"Webhook {webhook_id} returned client error {status_code}: {response_text[:200]}" | ||
| ) | ||
| return | ||
| else: | ||
| # Server errors (5xx) or other - retry | ||
| logger.warning(f"Webhook {webhook_id} returned error {status_code}, will retry") | ||
| raise WebhookServiceException(f"Webhook returned status {status_code}") | ||
|
|
Contributor
There was a problem hiding this comment.
Retry policy drops 408/429 responses.
Treat 408/429 as retriable; otherwise rate limits/timeouts will silently drop events.
🛠️ Suggested fix
- elif 400 <= status_code < 500:
- # Client errors (4xx) - don't retry, log and exit
+ elif 400 <= status_code < 500 and status_code not in {408, 429}:
+ # Non-retriable client errors (4xx)
logger.warning(
f"Webhook {webhook_id} returned client error {status_code}: {response_text[:200]}"
)
return
- else:
- # Server errors (5xx) or other - retry
+ else:
+ # Server errors (5xx) + retriable client codes (408/429)
logger.warning(f"Webhook {webhook_id} returned error {status_code}, will retry")
raise WebhookServiceException(f"Webhook returned status {status_code}")🤖 Prompt for AI Agents
In `@echo/server/dembrane/tasks.py` around lines 904 - 918, The current logic
treats all 4xx codes as non-retriable which incorrectly drops 408 and 429;
update the webhook response handling in the function that uses webhook_id,
status_code, logger and raises WebhookServiceException so that 408 (Request
Timeout) and 429 (Too Many Requests) are treated as retriable: either add an
explicit branch that checks if status_code in (408, 429) and logs a warning then
raise WebhookServiceException, or change the existing 400 <= status_code < 500
branch to exclude 408 and 429 (e.g., 400 <= status_code < 500 and status_code
not in (408,429)); keep non-retriable 4xx logging for other client errors and
preserve the 2xx success return path.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Commits
Key Changes
New Files