feat: add idempotent SSE onboarding for Buddy Assist - #976
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds an authenticated SSE Assist onboarding endpoint. It validates inputs and tenant scope, builds personalized templates, provisions widgets, serializes merchant onboarding with Redis, and reports progress or errors through structured events. ChangesAssist onboarding
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🟡 Changes recommended
The current onboarding logic has correctness issues that can break idempotency and/or accept ambiguous inputs, which can lead to inconsistent provisioning outcomes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds a new Server-Sent Events (SSE) onboarding workflow to provision/refresh the backend-owned “Buddy Assist” template + widget_config for a merchant, including optional prompt personalization via the existing website scraper service.
Changes:
- Introduces
build_assist_template()to generate a canonical Buddy Assist template (with optional personalization + prompt hashing). - Adds Pydantic schemas for Assist onboarding request/complete SSE payloads with “public name” normalization.
- Implements a new
/assist/onboard/streamSSE endpoint that locks per (reseller, merchant) and creates/updates the template + widget_config.
File summaries
| File | Description |
|---|---|
| app/services/breeze_buddy/assist_template.py | New backend-owned Assist template builder (scrape-driven personalization + canonical prompt/functions/config). |
| app/schemas/breeze_buddy/assist_onboarding.py | New request/response schemas for SSE onboarding with alias normalization and OpenAPI shaping. |
| app/api/routers/breeze_buddy/assist_onboarding/init.py | New SSE onboarding endpoint with Redis locking, idempotent create/update logic, and cleanup on failure. |
| app/api/routers/breeze_buddy/init.py | Registers the new assist onboarding router. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| # A merchant may already have a widget backed by a manually-managed | ||
| # template. The first Assist onboarding must not overwrite it. Create | ||
| # an Assist template and repoint the widget; subsequent runs will then | ||
| # recognize and update that same Assist template. | ||
| if not existing.name.startswith("buddy-assist-agent-"): | ||
| existing = None |
| if not isinstance(value, dict): | ||
| return value | ||
| normalized = dict(value) | ||
| for field_name, public_name in _REQUEST_ALIASES.items(): | ||
| if public_name in normalized: | ||
| normalized[field_name] = normalized[public_name] | ||
| return normalized |
| updated = await update_widget_config( | ||
| widget_config.id, | ||
| template_id=template_id, | ||
| allowed_origins=body.allowed_origins or widget_config.allowed_origins, | ||
| active=body.is_active, |
| "expected_fields": { | ||
| "email": {"value": "email", "source": "llm"}, | ||
| "phone": {"value": "phone", "source": "llm"}, | ||
| "shopDomain": {"value": "{shop_url}", "source": "static"}, | ||
| "orderNumber": {"value": "orderNumber", "source": "llm"}, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
app/services/breeze_buddy/assist_template.py (1)
34-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAccept the validated request model instead of an untyped dict.
The only caller builds this
paramsdict from an already-validatedAssistOnboardingStreamRequest, which has resolved every camelCase alias. This function then repeats the alias resolution for eight keys. The duplication means a schema field rename fails silently here instead of at type-check time. AcceptAssistOnboardingStreamRequest(or an explicit keyword signature) and drop the camelCase fallbacks.🤖 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 `@app/services/breeze_buddy/assist_template.py` around lines 34 - 60, Update build_assist_template to accept the validated AssistOnboardingStreamRequest instead of Dict[str, Any), or use an explicit keyword signature, and access its normalized snake_case fields directly. Remove the camelCase alias fallbacks and duplicate params resolution from build_assist_template, while preserving the existing template-building behavior.app/schemas/breeze_buddy/assist_onboarding.py (1)
79-84: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAlign the rate-limit field names with the camelCase wire contract.
allowed_originsuses the aliasallowedOrigins, but the four limit fields below it have no alias. Withmodel_dump(by_alias=True, ...)in the router, thecompleteevent mixesallowedOriginswithmax_sessions_per_ip_hour. Every progress event inapp/api/routers/breeze_buddy/assist_onboarding/__init__.pyuses camelCase. Add aliases so the completion payload is consistent for clients.♻️ Proposed alias additions
- max_sessions_per_ip_hour: int - max_messages_per_ip_hour: int - max_concurrent_per_ip: int - max_voice_sessions_per_ip_hour: int + max_sessions_per_ip_hour: int = Field(alias="maxSessionsPerIpHour") + max_messages_per_ip_hour: int = Field(alias="maxMessagesPerIpHour") + max_concurrent_per_ip: int = Field(alias="maxConcurrentPerIp") + max_voice_sessions_per_ip_hour: int = Field(alias="maxVoiceSessionsPerIpHour")🤖 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 `@app/schemas/breeze_buddy/assist_onboarding.py` around lines 79 - 84, Update the rate-limit fields in the onboarding schema, alongside allowed_origins in the relevant model, to define camelCase aliases for max_sessions_per_ip_hour, max_messages_per_ip_hour, max_concurrent_per_ip, and max_voice_sessions_per_ip_hour. Ensure model_dump(by_alias=True) produces the same camelCase names used by progress events and the existing allowedOrigins field.
🤖 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 `@app/api/routers/breeze_buddy/assist_onboarding/__init__.py`:
- Around line 317-320: Move the four widget rate-limit defaults from the
router’s inline values into app/core/config/static.py as SCREAMING_SNAKE_CASE
configuration constants loaded with get_required_env(), then update the
rate-limit setup in the onboarding router to reference those constants instead
of hardcoded numbers.
- Around line 88-91: The onboarding stream needs bounded semaphore waiting and
keepalive output during the scrape. Update the acquisition around
_onboarding_stream_semaphore to use asyncio.wait_for with the existing semaphore
timeout strategy, emitting a busy error event if it expires (or a queued
progress event before waiting); also add periodic SSE comment heartbeats while
build_assist_template performs its scrape between personalizing_prompt and
prompt_ready.
- Around line 291-302: Update _create_or_update_widget_config and
_get_existing_widget_config to return Optional[WidgetConfigResponse], and
replace _create_or_update_widget_config’s existing_widget: Optional[Any] with
the appropriate Optional[WidgetConfigResponse] type. Change the allowed-origins
schema field to Optional[List[str]] with a None default, then update the merge
logic to fall back to widget_config.allowed_origins only when
body.allowed_origins is None, preserving an explicit empty list.
- Around line 360-361: Update _onboarding_lock_key to normalize reseller_id and
merchant_id before composing the Redis lock key, using a collision-safe
fixed-length digest or separator-safe encoding for each identifier so distinct
tenant pairs cannot produce the same key while preserving the existing key
structure.
- Around line 350-357: Update _cleanup_created_template to catch exceptions
raised by delete_template_if_not_referenced, log the cleanup failure including
the exception details, and return False so callers’ original onboarding errors
can continue to the error-event yield.
In `@app/schemas/breeze_buddy/assist_onboarding.py`:
- Line 65: Update the onboarding schema’s allowed_origins field and its
validation flow to enforce the configured per-origin CORS length limit and a
maximum number of entries before create_widget_config or update_widget_config
persists data. Validate that every entry is a string and reject oversized items
or lists using the existing Pydantic validation patterns and configuration
constants.
In `@app/services/breeze_buddy/assist_template.py`:
- Around line 272-280: The Assist template naming contract must use one shared
prefix so producer and consumer support idempotent onboarding. In
app/services/breeze_buddy/assist_template.py lines 272-280, export
ASSIST_TEMPLATE_NAME_PREFIX and update _template_name to prepend it to the slug
instead of appending -buddy-assist; in
app/api/routers/breeze_buddy/assist_onboarding/__init__.py lines 236-241, import
that constant and use it in the startswith check instead of the literal prefix.
- Around line 304-341: Move the hardcoded WISMO and tracking service URLs, plus
the model, region, and token-limit settings used by `_base_configurations` and
`_shopify_configurations`, into `app/core/config/static.py` loaded via
`get_required_env()`. Update the template definitions in `assist_template.py` to
reference the centralized configuration values, including the endpoints in
`http_request`, and remove the embedded production-specific literals.
---
Nitpick comments:
In `@app/schemas/breeze_buddy/assist_onboarding.py`:
- Around line 79-84: Update the rate-limit fields in the onboarding schema,
alongside allowed_origins in the relevant model, to define camelCase aliases for
max_sessions_per_ip_hour, max_messages_per_ip_hour, max_concurrent_per_ip, and
max_voice_sessions_per_ip_hour. Ensure model_dump(by_alias=True) produces the
same camelCase names used by progress events and the existing allowedOrigins
field.
In `@app/services/breeze_buddy/assist_template.py`:
- Around line 34-60: Update build_assist_template to accept the validated
AssistOnboardingStreamRequest instead of Dict[str, Any), or use an explicit
keyword signature, and access its normalized snake_case fields directly. Remove
the camelCase alias fallbacks and duplicate params resolution from
build_assist_template, while preserving the existing template-building behavior.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 3c814677-a579-40aa-9f77-05fd715ecdf8
📒 Files selected for processing (4)
app/api/routers/breeze_buddy/__init__.pyapp/api/routers/breeze_buddy/assist_onboarding/__init__.pyapp/schemas/breeze_buddy/assist_onboarding.pyapp/services/breeze_buddy/assist_template.py
| async with _onboarding_stream_semaphore: | ||
| async with lock: | ||
| yield _event("progress", step="personalizing_prompt", status="running") | ||
| existing_widget = await _get_existing_widget_config(body) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The stream stalls silently while it waits for the semaphore, and it sends no keepalive during the scrape.
Two gaps affect the client experience on this path.
async with _onboarding_stream_semaphore has no timeout. Request nine and later block here after emitting only started and validating_request. The client sees an open stream with no events and no reason. Add a bounded wait with asyncio.wait_for and emit a busy error event when the wait expires, or emit a queued progress event before acquiring.
build_assist_template awaits a scrape with an 18-second timeout at Line 92. No event is written between personalizing_prompt and prompt_ready. Proxies that apply an idle timeout to SSE connections can drop the stream. Send a periodic SSE comment line as a heartbeat during the scrape.
🤖 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 `@app/api/routers/breeze_buddy/assist_onboarding/__init__.py` around lines 88 -
91, The onboarding stream needs bounded semaphore waiting and keepalive output
during the scrape. Update the acquisition around _onboarding_stream_semaphore to
use asyncio.wait_for with the existing semaphore timeout strategy, emitting a
busy error event if it expires (or a queued progress event before waiting); also
add periodic SSE comment heartbeats while build_assist_template performs its
scrape between personalizing_prompt and prompt_ready.
| async def _create_or_update_widget_config( | ||
| body: AssistOnboardingStreamRequest, | ||
| template_id: str, | ||
| existing_widget: Optional[Any] = None, | ||
| ): | ||
| widget_config = existing_widget | ||
|
|
||
| if widget_config: | ||
| updated = await update_widget_config( | ||
| widget_config.id, | ||
| template_id=template_id, | ||
| allowed_origins=body.allowed_origins or widget_config.allowed_origins, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the missing return type annotations and preserve an explicit empty origin list.
Two items on this helper.
_create_or_update_widget_config at Line 295 and _get_existing_widget_config at Line 328 declare no return type. The coding guidelines require type hints on all function signatures. create_widget_config in app/database/accessor/breeze_buddy/widget_config.py returns Optional[WidgetConfigResponse], so annotate both helpers with that type and replace Optional[Any] on the existing_widget parameter.
Line 302 uses body.allowed_origins or widget_config.allowed_origins. A caller who sends allowedOrigins: [] to clear the list keeps the stored value instead. The schema default is also an empty list, so an omitted field and an explicit empty list are indistinguishable here. If clearing must be supported, make the field Optional[List[str]] with a None default and test for None.
Based on learnings from the coding guidelines: "Include required type hints on all function signatures".
🤖 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 `@app/api/routers/breeze_buddy/assist_onboarding/__init__.py` around lines 291
- 302, Update _create_or_update_widget_config and _get_existing_widget_config to
return Optional[WidgetConfigResponse], and replace
_create_or_update_widget_config’s existing_widget: Optional[Any] with the
appropriate Optional[WidgetConfigResponse] type. Change the allowed-origins
schema field to Optional[List[str]] with a None default, then update the merge
logic to fall back to widget_config.allowed_origins only when
body.allowed_origins is None, preserving an explicit empty list.
Source: Coding guidelines
| max_sessions_per_ip_hour=60, | ||
| max_messages_per_ip_hour=600, | ||
| max_concurrent_per_ip=4, | ||
| max_voice_sessions_per_ip_hour=10, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move the widget rate-limit defaults into central configuration.
The four abuse-control limits are magic numbers in the router. They govern public widget traffic, so they must be tunable per environment without a code change. The coding guidelines require configuration to load from app/core/config/static.py. Define them as SCREAMING_SNAKE_CASE constants there and reference them here.
Based on learnings from the coding guidelines: "Load ALL configuration from app/core/config/static.py using get_required_env() for mandatory variables".
🤖 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 `@app/api/routers/breeze_buddy/assist_onboarding/__init__.py` around lines 317
- 320, Move the four widget rate-limit defaults from the router’s inline values
into app/core/config/static.py as SCREAMING_SNAKE_CASE configuration constants
loaded with get_required_env(), then update the rate-limit setup in the
onboarding router to reference those constants instead of hardcoded numbers.
Source: Coding guidelines
| async def _cleanup_created_template(template_id: str) -> bool: | ||
| if await delete_template_if_not_referenced(template_id): | ||
| return True | ||
| logger.warning( | ||
| "Could not clean up Assist template after onboarding failure", | ||
| template_id=template_id, | ||
| ) | ||
| return False |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the cleanup call so a cleanup failure does not suppress the error event.
_cleanup_created_template does not catch exceptions from delete_template_if_not_referenced. Callers invoke it from inside the except handlers at Lines 185, 194, and 204, before the yield that writes the error event. If the delete raises, the new exception replaces the original one, the generator terminates, and the client receives a truncated stream with no error event and no explanation.
Catch and log inside the helper, and return False.
🛡️ Proposed fix
async def _cleanup_created_template(template_id: str) -> bool:
- if await delete_template_if_not_referenced(template_id):
- return True
+ try:
+ if await delete_template_if_not_referenced(template_id):
+ return True
+ except Exception:
+ logger.exception(
+ "Assist template cleanup raised after onboarding failure",
+ template_id=template_id,
+ )
+ return False
logger.warning(
"Could not clean up Assist template after onboarding failure",
template_id=template_id,
)
return 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.
| async def _cleanup_created_template(template_id: str) -> bool: | |
| if await delete_template_if_not_referenced(template_id): | |
| return True | |
| logger.warning( | |
| "Could not clean up Assist template after onboarding failure", | |
| template_id=template_id, | |
| ) | |
| return False | |
| async def _cleanup_created_template(template_id: str) -> bool: | |
| try: | |
| if await delete_template_if_not_referenced(template_id): | |
| return True | |
| except Exception: | |
| logger.exception( | |
| "Assist template cleanup raised after onboarding failure", | |
| template_id=template_id, | |
| ) | |
| return False | |
| logger.warning( | |
| "Could not clean up Assist template after onboarding failure", | |
| template_id=template_id, | |
| ) | |
| return False |
🤖 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 `@app/api/routers/breeze_buddy/assist_onboarding/__init__.py` around lines 350
- 357, Update _cleanup_created_template to catch exceptions raised by
delete_template_if_not_referenced, log the cleanup failure including the
exception details, and return False so callers’ original onboarding errors can
continue to the error-event yield.
| def _onboarding_lock_key(reseller_id: str, merchant_id: str) -> str: | ||
| return f"breeze_buddy:assist_onboarding:{reseller_id}:{merchant_id}:lock" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect RedisLock and the repository namespace convention.
set -euo pipefail
fd -t f 'locks.py' app/services/redis --exec cat -n {}
rg -n -C 3 'namespace=' --glob '*.py' | head -50Repository: juspay/clairvoyance
Length of output: 6875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
fd -t f 'redis.*\.py| locks.*\.py' app app/services | sed -n '1,120p'
fd -t f '__init__.py|router.*\.py|.*onboarding.*\.py' app/api/routers | sed -n '1,160p'
echo
echo "== inspect target router section =="
FILE="$(fd -t f '__init__.py' app/api/routers/breeze_buddy/assist_onboarding | head -n 1 || true)"
if [ -n "${FILE:-}" ]; then
wc -l "$FILE"
sed -n '330,375p' "$FILE" | cat -n
else
echo "target file not found"
fi
echo
echo "== RedisService/client signatures =="
rg -n -C 4 'class RedisService|def get_redis_service|def redis_get|def redis_set|def set\(|def get\(|namespace' app --glob '*.py' | sed -n '1,220p'Repository: juspay/clairvoyance
Length of output: 21970
Normalize the onboarding lock key from the tenant identifiers.
_onboarding_lock_key() builds breeze_buddy:assist_onboarding:{reseller_id}:{merchant_id}:lock, but reseller_id and merchant_id are unescaped values. Keys like ("a:b", "c") and ("a", "b:c") collide under RedisLock, causing one authorized tenant to block and get a spurious ASSIST_ONBOARDING_IN_PROGRESS conflict. Use a fixed-length digest for each identifier, or encode the separator.
🤖 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 `@app/api/routers/breeze_buddy/assist_onboarding/__init__.py` around lines 360
- 361, Update _onboarding_lock_key to normalize reseller_id and merchant_id
before composing the Redis lock key, using a collision-safe fixed-length digest
or separator-safe encoding for each identifier so distinct tenant pairs cannot
produce the same key while preserving the existing key structure.
Source: Coding guidelines
| "defaults to `google`." | ||
| ), | ||
| ) | ||
| allowed_origins: List[str] = Field(default_factory=list) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace allowed_origins validation and enforcement.
set -euo pipefail
rg -n -C 5 'allowed_origins' --glob '*.py'
rg -n -C 3 -P '\borigin\b.*(validate|valid_|check)' --glob '*.py'Repository: juspay/clairvoyance
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked Python files containing 'allowed_origins' =="
git ls-files '*.py' | xargs rg -n -C 6 'allowed_origins|allowedOrigins' || true
echo
echo "== tracked Python files containing origin validators =="
git ls-files '*.py' | xargs rg -n -C 3 'origin|Origin|cors|CORS|validate.*origin|origins' || true
echo
echo "== schema file =="
fd -a 'assist_onboarding.py' . | while read -r f; do echo "--- $f"; sed -n '1,120p' "$f" | cat -n; doneRepository: juspay/clairvoyance
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant schema declarations =="
sed -n '1,90p' app/schemas/breeze_buddy/assist_onboarding.py | cat -n
sed -n '1,80p' app/schemas/breeze_buddy/widget_config.py | cat -n
echo
echo "== model validators/field validators in relevant schemas =="
git ls-files 'app/schemas/**/*.py' 'app/api/routers/breeze_buddy/assist_onboarding/**' 'app/services/**/*.py' | xargs rg -n 'allowed_origins|allowedOrigins|`@model_validator`|`@field_validator`|validator|Field\(' | head -n 200
echo
echo "== test assertions around widget_config.allowed_origins usage =="
grep -n -C 4 'allowed_origins' tests/app tests || trueRepository: juspay/clairvoyance
Length of output: 26879
Security Misconfiguration (CWE-942)
Reachability: External · Exploitability: Moderate
Reachability path
● Entry
app/api/routers/breeze_buddy/__init__.py
│
▼
● Hop
app/api/routers/breeze_buddy/assist_onboarding/__init__.py:50
onboard_assist_stream: Create or refresh an Assist template and widget config over SSE.
│
▼
● Sink
app/schemas/breeze_buddy/assist_onboarding.py
Bound and validate allowed_origins before persistence.
allowed_origins is sent from the onboarding request into _normalize_shop_url and into the widget allowlist, with only string-item and no list-size validation. Reject entries larger than the configured per-item CORS limit and limit the list size (for example with a Pydantic Field(max_length=...) and a field/model validator) before create_widget_config/update_widget_config.
🤖 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 `@app/schemas/breeze_buddy/assist_onboarding.py` at line 65, Update the
onboarding schema’s allowed_origins field and its validation flow to enforce the
configured per-origin CORS length limit and a maximum number of entries before
create_widget_config or update_widget_config persists data. Validate that every
entry is a string and reject oversized items or lists using the existing
Pydantic validation patterns and configuration constants.
Source: Linters/SAST tools
| def _template_name(shop_name: str, merchant_id: str) -> str: | ||
| # Tenant-bound merchant ids are storefront domains in this workflow. The | ||
| # first DNS label gives stable names such as amirandsons-buddy-assist even | ||
| # when the prompt contents change after a fresh personalization scrape. | ||
| merchant_host = merchant_id.split("://", 1)[-1].split("/", 1)[0] | ||
| merchant_key = merchant_host.split(".", 1)[0] | ||
| naming_source = shop_name.strip() or merchant_key | ||
| slug = re.sub(r"[^a-z0-9]+", "-", naming_source.lower()).strip("-") | ||
| return f"{slug or 'store'}-buddy-assist" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
The Assist template naming contract is inconsistent between the producer and the consumer, so onboarding is not idempotent. _template_name emits <slug>-buddy-assist, and the router recognizes an Assist-managed template by the buddy-assist-agent- prefix. No generated name satisfies that test, so every re-onboarding creates a new template row, orphans the previous one, and leaves the replace_template update branch unreachable. This contradicts the stated idempotency objective of the PR.
app/services/breeze_buddy/assist_template.py#L272-L280: export a module-levelASSIST_TEMPLATE_NAME_PREFIXconstant and returnf"{ASSIST_TEMPLATE_NAME_PREFIX}{slug or 'store'}"instead of appending the-buddy-assistsuffix.app/api/routers/breeze_buddy/assist_onboarding/__init__.py#L236-L241: importASSIST_TEMPLATE_NAME_PREFIXand replace the"buddy-assist-agent-"string literal in thestartswithcheck with that constant.
📍 Affects 2 files
app/services/breeze_buddy/assist_template.py#L272-L280(this comment)app/api/routers/breeze_buddy/assist_onboarding/__init__.py#L236-L241
🤖 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 `@app/services/breeze_buddy/assist_template.py` around lines 272 - 280, The
Assist template naming contract must use one shared prefix so producer and
consumer support idempotent onboarding. In
app/services/breeze_buddy/assist_template.py lines 272-280, export
ASSIST_TEMPLATE_NAME_PREFIX and update _template_name to prepend it to the slug
instead of appending -buddy-assist; in
app/api/routers/breeze_buddy/assist_onboarding/__init__.py lines 236-241, import
that constant and use it in the startswith check instead of the literal prefix.
| "http_request": { | ||
| "url": "https://nautilus.breezelabs.app/apps/breeze-buddy/api/wismo/order", | ||
| "auth": {"type": "bearer", "token": "{wismo_secret}"}, | ||
| "method": "GET", | ||
| "timeout": 15, | ||
| "query_params": { | ||
| "email": "{email}", | ||
| "phone": "{phone}", | ||
| "shopDomain": "{shopDomain}", | ||
| "orderNumber": "{orderNumber}", | ||
| }, | ||
| }, | ||
| "expected_fields": { | ||
| "email": {"value": "email", "source": "llm"}, | ||
| "phone": {"value": "phone", "source": "llm"}, | ||
| "shopDomain": {"value": "{shop_url}", "source": "static"}, | ||
| "orderNumber": {"value": "orderNumber", "source": "llm"}, | ||
| "wismo_secret": {"value": "{wismo_secret}", "source": "static"}, | ||
| }, | ||
| "expected_response_schema": "full", | ||
| }, | ||
| { | ||
| "name": "read_page_content", | ||
| "type": "http", | ||
| "required": ["url"], | ||
| "properties": { | ||
| "url": { | ||
| "type": "string", | ||
| "description": "A tracking_url returned by get_order_status in this session.", | ||
| } | ||
| }, | ||
| "description": "Read a courier tracking page returned by get_order_status to find delivery details.", | ||
| "http_request": { | ||
| "url": "https://r.jina.ai/{url}", | ||
| "method": "GET", | ||
| "timeout": 30, | ||
| "max_retries": 1, | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move the hardcoded service endpoints into central configuration.
Lines 305 and 337 embed production hostnames in the template definition. _shopify_configurations adds two more at Lines 438 and 446, and _base_configurations fixes the model, region, and token limits at Lines 422-426. A non-production deployment generates templates that call the production WISMO endpoint at nautilus.breezelabs.app.
The coding guidelines require all configuration to load from app/core/config/static.py through get_required_env(). Move these endpoints and model settings there and reference the constants here.
Based on learnings from the coding guidelines: "Load ALL configuration from app/core/config/static.py using get_required_env() for mandatory variables; never import directly from os.environ elsewhere".
🤖 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 `@app/services/breeze_buddy/assist_template.py` around lines 304 - 341, Move
the hardcoded WISMO and tracking service URLs, plus the model, region, and
token-limit settings used by `_base_configurations` and
`_shopify_configurations`, into `app/core/config/static.py` loaded via
`get_required_env()`. Update the template definitions in `assist_template.py` to
reference the centralized configuration values, including the endpoints in
`http_request`, and remove the embedded production-specific literals.
Source: Coding guidelines
|
|
||
|
|
||
| _CANONICAL_OPERATING_PRINCIPLES = r""" | ||
| ## Operating principles |
There was a problem hiding this comment.
do we need to hardcode this ? cant we create a default template and create duplicates for every onborading ?
f99ed1c to
0b2916e
Compare
3e02a64 to
487f1ea
Compare
| _SCRAPE_TIMEOUT_SECONDS = 18 | ||
| _MAX_BRAND_CONTEXT_CHARS = 24_000 | ||
|
|
||
| _ONBOARDING_SCRAPE_PROMPT = """Visit the supplied storefront and create concise, |
There was a problem hiding this comment.
this is generic to shopping assistant, so don't you think this should go into commerce folder?
and SHOPIFY_MCP_SERVER_NAME = "shopify-storefront" all these are related to shopping commerce.
487f1ea to
cc801f0
Compare
|
|
||
| reseller_id: str = Field(..., min_length=1, max_length=255) | ||
| merchant_id: str = Field(..., min_length=1, max_length=255) | ||
| shop_name: str = Field(..., min_length=1, max_length=255) |
| reseller_id: str = Field(..., min_length=1, max_length=255) | ||
| merchant_id: str = Field(..., min_length=1, max_length=255) | ||
| shop_name: str = Field(..., min_length=1, max_length=255) | ||
| shop_url: str = Field(..., max_length=2048) |
| is_shopify: bool | ||
| allowed_origins: List[str] = Field(..., max_length=20) | ||
| provider: Literal["google"] = "google" | ||
| brand_name: Optional[str] = Field(None, min_length=1, max_length=255) |
cc801f0 to
54f75bc
Compare
Dev proof:
feat: add idempotent SSE onboarding for Buddy Assist
Summary by CodeRabbit