Skip to content

refactor(api): isolate trial preview queries and admission - #42644

Open
hjlarry wants to merge 1 commit into
refactor/trial-suggested-questionsfrom
refactor/trial-preview
Open

hjlarry wants to merge 1 commit into
refactor/trial-suggested-questionsfrom
refactor/trial-preview

Conversation

@hjlarry

@hjlarry hjlarry commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Trial parameters, site, and dataset previews now share detached app admission and injected queries instead of carrying a controller-owned ORM session through the response. Preview admission still accepts a Trial registration or recommended-catalog membership independently of login, the trial feature switch, and account quotas.

Site and dataset queries return detached records scoped to the admitted app owner; parameters reuse the existing app-definition service. Catalog I/O and response icon signing run outside the preview repository sessions. Dataset IDs, duplicate handling, and the legacy page/limit metadata behavior are preserved.

Site errors now identify an unavailable site (app_site_unavailable, 403) or unavailable owner (app_owner_unavailable, 403). A missing owner previously failed an assertion with 500. App detail and workflow detail retain their existing ORM path for the next migration.

Stacked on #42635; merge that PR first.

@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 64.12% 64.18% +0.06%
Strict coverage 63.73% 63.79% +0.06%
Typed symbols 46,008 46,110 +102
Untyped symbols 25,900 25,893 -7
Modules 3407 3412 +5

@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-09-21 07:02:43.491828707 +0000
+++ /tmp/pyrefly_pr.txt	2026-09-21 07:02:30.295728360 +0000
@@ -3438,10 +3438,8 @@
   --> tests/unit_tests/controllers/console/explore/test_message.py:66:22
 ERROR Argument `Literal['normal']` is not assignable to parameter `value` with type `MessageStatus | SQLCoreOperations[MessageStatus]` in function `sqlalchemy.orm.base.Mapped.__set__` [bad-argument-type]
   --> tests/unit_tests/controllers/console/explore/test_saved_message.py:38:22
-ERROR Argument `Literal['uuid']` is not assignable to parameter `customize_token_strategy` with type `CustomizeTokenStrategy | SQLCoreOperations[CustomizeTokenStrategy]` in function `models.model.Site.__init__` [bad-argument-type]
-  --> tests/unit_tests/controllers/console/explore/test_trial.py:87:34
 ERROR Object of class `FunctionType` has no attribute `assert_called_once_with` [missing-attribute]
-   --> tests/unit_tests/controllers/console/explore/test_trial.py:182:5
+   --> tests/unit_tests/controllers/console/explore/test_trial.py:116:5
 ERROR Argument `() -> Session` is not assignable to parameter `session_factory` with type `sessionmaker[@_]` in function `sqlalchemy.orm.scoping.scoped_session.__init__` [bad-argument-type]
   --> tests/unit_tests/controllers/console/explore/test_wraps.py:28:39
 ERROR Argument `str` is not assignable to parameter `view` with type `(InstalledApp) -> str` in function `controllers.console.explore.wraps.decorator` [bad-argument-type]

@hjlarry
hjlarry added this pull request to stack #42630 September 21, 2026 07:03
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.86207% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.32%. Comparing base (0457d27) to head (38a8ac6).

Files with missing lines Patch % Lines
api/controllers/console/app/preview_admission.py 88.88% 1 Missing and 1 partial ⚠️
api/controllers/console/explore/trial.py 92.00% 2 Missing ⚠️
api/services/app_preview_query_service.py 96.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                         Coverage Diff                          @@
##           refactor/trial-suggested-questions   #42644    +/-   ##
====================================================================
  Coverage                               87.32%   87.32%            
====================================================================
  Files                                    5449     5452     +3     
  Lines                                  313783   313895   +112     
  Branches                                63130    63137     +7     
====================================================================
+ Hits                                   274007   274119   +112     
  Misses                                  34445    34445            
  Partials                                 5331     5331            
Flag Coverage Δ
api 87.03% <95.86%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant