UN-3476 [FIX] Revert atomic wrap on set_user_organization#1977
Merged
Conversation
The atomic wrap from #1954 uncommits the new org row when frictionless_onboarding HTTP-calls the LLMW portal mid-transaction. The portal runs on a separate DB session and under READ COMMITTED cannot see the uncommitted row, so the call returns 400 and the caller silently persists an adapter with an empty unstract_key. Every new signup since 2026-05-19 09:47 UTC ships a broken free-trial X2Text adapter (401 on first OCR). Hotfix only — Phase 2 (UN-3476) restructures the function so the atomic guarantee is reapplied around just the pure-DB writes, with HTTP and non-DB side effects moved outside the transaction. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Contributor
WalkthroughThis PR implements a retry mechanism for frictionless onboarding when free-trial adapter provisioning is incomplete. The backend refactors organization setup transaction scoping and unconditionally runs frictionless onboarding with robust error handling. The frontend adds a retry UI with state tracking, adapter refresh logic, and a conditional Alert banner. ChangesFrictionless Onboarding Retry Feature
Sequence DiagramssequenceDiagram
participant Client
participant SetUserOrg as set_user_organization
participant OrgService as OrganizationService
participant AuthService as auth_service
participant DB as Database
Client->>SetUserOrg: request to set organization
SetUserOrg->>OrgService: create_organization()
OrgService->>DB: [atomic] write org + tenant user
DB-->>OrgService: committed rows
OrgService-->>SetUserOrg: org created
SetUserOrg->>AuthService: frictionless_onboarding(org,user)
AuthService-->>SetUserOrg: success or logged error
SetUserOrg->>Client: org setup complete
sequenceDiagram
participant User
participant OnBoard as OnBoard component
participant API as /api/v1/users/profile/retry-frictionless-setup
participant AdapterAPI as /api/v1/unstract/adapter
participant SessionStore as Session Store
User->>OnBoard: click Retry button
OnBoard->>API: POST retry request
API-->>OnBoard: success or error
OnBoard->>AdapterAPI: GET adapter types
AdapterAPI-->>OnBoard: adapter list
OnBoard->>SessionStore: persist updated adapters
OnBoard->>OnBoard: update UI state (complete or warning)
OnBoard-->>User: dismiss or show banner
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/account_v2/authentication_controller.py`:
- Around line 190-204: The current except block around transaction.atomic() maps
any IntegrityError to an organization-duplicate error; narrow the handling so
IntegrityErrors from OrganizationService.create_organization are converted to
DuplicateData with the organization message but IntegrityErrors from
self.create_tenant_user are handled differently (either raise a member-specific
DuplicateData or re-raise the original IntegrityError). Concretely, isolate the
call to OrganizationService.create_organization inside its own try/except
IntegrityError and raise the organization DuplicateData there, then call
self.create_tenant_user inside the same transaction but without catching all
IntegrityError into the organization message (or catch and raise a
member-specific error) so the true cause is preserved.
In `@frontend/src/components/onboard/OnBoard.jsx`:
- Around line 42-44: The current UI derives showFrictionlessRetry from
adaptersList.length which can misidentify manually-connected orgs as having an
interrupted free-trial; replace this heuristic with an explicit server-supplied
flag (e.g., use a prop or org state such as isFrictionlessProvisioning,
freeTrialProvisioning, or provisioningStatus) and compute showFrictionlessRetry
from that flag instead of adaptersList.length; update the OnBoard.jsx logic that
sets showFrictionlessRetry and any consumers that call the retry endpoint
(referencing showFrictionlessRetry and adaptersList) so the banner and retry
action are only shown when the server indicates interrupted frictionless
onboarding.
🪄 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: CHILL
Plan: Pro
Run ID: 82ea679f-4728-41ae-88dc-8939556160fb
📒 Files selected for processing (3)
backend/account_v2/authentication_controller.pyfrontend/src/components/onboard/OnBoard.jsxfrontend/src/components/onboard/onBoard.css
Contributor
|
| Filename | Overview |
|---|---|
| backend/account_v2/authentication_controller.py | Removes @transaction.atomic decorator and its import — a precise two-line revert that restores pre-#1954 commit ordering so the org row is visible to the portal before the HTTP call fires. |
Sequence Diagram
sequenceDiagram
participant Client
participant set_user_organization
participant LocalDB as Local DB (Django session)
participant Portal as LLM Whisperer Portal
participant PortalDB as Portal DB (separate session)
Note over set_user_organization,LocalDB: BEFORE this PR (#1954 broken state)
Client->>set_user_organization: login / org selection
set_user_organization->>LocalDB: BEGIN (transaction.atomic)
set_user_organization->>LocalDB: INSERT org row (uncommitted)
set_user_organization->>Portal: POST /onboarding-setup/
Portal->>PortalDB: SELECT org row
PortalDB-->>Portal: row not visible (uncommitted)
Portal-->>set_user_organization: 400 error
set_user_organization->>LocalDB: writes empty unstract_key
set_user_organization->>LocalDB: COMMIT
Note right of LocalDB: org saved with empty key
Note over set_user_organization,LocalDB: AFTER this PR (reverted / correct state)
Client->>set_user_organization: login / org selection
set_user_organization->>LocalDB: INSERT org row
set_user_organization->>LocalDB: COMMIT (auto-commit)
set_user_organization->>Portal: POST /onboarding-setup/
Portal->>PortalDB: SELECT org row
PortalDB-->>Portal: row visible (committed)
Portal-->>set_user_organization: 200 + subscription key
set_user_organization->>LocalDB: writes valid unstract_key
Reviews (4): Last reviewed commit: "Merge branch 'main' into fix/un-3476-fri..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
chandrasekharan-zipstack
force-pushed
the
fix/un-3476-frictionless
branch
from
1cf1847 to
86331f1
Compare
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/onboard/OnBoard.jsx (1)
42-44:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUse an explicit provisioning flag, not adapter count, to gate retry UI.
Line 42-Line 44 still infer interrupted free-trial from adapter count, which can misclassify manually configured orgs and call the retry endpoint incorrectly. Gate this on a backend-provided provisioning/free-trial status instead.
🤖 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 `@frontend/src/components/onboard/OnBoard.jsx` around lines 42 - 44, The UI currently infers interrupted free-trial from adaptersList length via the showFrictionlessRetry constant; replace this heuristic with an explicit backend-provided flag (e.g., freeTrialStatus or isProvisioning) passed into OnBoard.jsx (props or derived from API/state) and use that flag to determine whether to show the retry UI and call the retry endpoint; update references to showFrictionlessRetry to read the new flag (and remove the adaptersList length check) and ensure any retry API call is only invoked when the backend flag indicates an interrupted provisioning/free-trial.
🤖 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 `@frontend/src/components/onboard/OnBoard.jsx`:
- Around line 57-59: The adapter type normalization can throw when items lack
adapter_type or it's not a string; update the adapterTypes creation (the code
that builds adapterTypes from data and obj.adapter_type) to defensively filter
and normalize only string values—e.g., skip null/undefined/non-string
adapter_type entries, convert remaining strings to lowercase, and dedupe via Set
so malformed API items don't cause runtime errors during retry recovery.
---
Duplicate comments:
In `@frontend/src/components/onboard/OnBoard.jsx`:
- Around line 42-44: The UI currently infers interrupted free-trial from
adaptersList length via the showFrictionlessRetry constant; replace this
heuristic with an explicit backend-provided flag (e.g., freeTrialStatus or
isProvisioning) passed into OnBoard.jsx (props or derived from API/state) and
use that flag to determine whether to show the retry UI and call the retry
endpoint; update references to showFrictionlessRetry to read the new flag (and
remove the adaptersList length check) and ensure any retry API call is only
invoked when the backend flag indicates an interrupted provisioning/free-trial.
🪄 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: CHILL
Plan: Pro
Run ID: f01bcaaf-88af-4334-bd25-4cc7376fcc23
📒 Files selected for processing (2)
frontend/src/components/onboard/OnBoard.jsxfrontend/src/components/onboard/onBoard.css
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/onboard/onBoard.css
chandrasekharan-zipstack
force-pushed
the
fix/un-3476-frictionless
branch
from
86331f1 to
d44bb75
Compare
chandrasekharan-zipstack
changed the title
pk-zipstack
approved these changes
May 21, 2026
athul-rs
approved these changes
May 21, 2026
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
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
3 participants
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.
What
Revert of the
@transaction.atomicwrap onAuthenticationController.set_user_organizationintroduced in PR #1954 (8b29fea02). Two-line diff: drop the decorator and its import.Why
PR #1954 caused a regression. Since
2026-05-19 09:47 UTC, every new signup on globe.unstract.com received a broken free-trial X2Text adapter (401 "Access denied due to invalid subscription key" on first OCR).Mechanism:
set_user_organizationinserts the org row, then makes an HTTP call to the LLM Whisperer portal (/onboarding-setup/) to provision the X2Text adapter's subscription key.unstract_key-> every subsequent OCR call gets 401.Removing the atomic wrap restores the pre-#1954 behaviour: the org INSERT commits before the portal call runs, so the portal can see it.
Why only the X2Text adapter breaks
The other three frictionless adapters (LLM, Vector DB, Embedding) are pure-local INSERTs from pre-baked env config, executed in the same DB session that holds the uncommitted org row - they read their own writes. Only X2Text crosses a process boundary to a service with an independent DB session, so only X2Text trips on the uncommitted-row visibility.
Scope
Revert only. The silent-swallow in
create_subscription_and_retrieve_key(which turns the portal 400 into an empty key written to the adapter row) is pre-existing brittleness and intentionally not addressed here - separate follow-up.Stranded orgs from the regression window (2002, 2003, 2004, 2006, 2007, 2008, 2009) are recoverable via the existing
setup_frictionless_onboardingmgmt command.Can this PR break any existing features?
No. It restores the exact pre-#1954 behaviour of
set_user_organization. The original orphan-org-on-portal-failure edge case PR #1954 was trying to address comes back, but that was tolerated for years pre-#1954 and is much rarer than the regression it caused.Database Migrations
None.
Env Config
None.
Related Issues or PRs
Notes on Testing
Checklist
I have read and understood the Contribution Guidelines.