-
Notifications
You must be signed in to change notification settings - Fork 446
Share batch operation in restate #2199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a server-side Restate transcription integration with three new /file-transcription endpoints (start, status, result), moves Restate client usage from the web client into the API, adjusts environment and dependencies, and adds web billing trial endpoints and UI integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Web as Web Server / Browser
participant API as API Server
participant Restate
rect rgb(220,240,255)
Note over Client,API: Start transcription
Client->>Web: user action -> start pipeline
Web->>API: POST /file-transcription/start (fileId, pipelineId) with Bearer
API->>API: validate auth & input, construct safeFileId
API->>Restate: submit SttFile workflow (ingress)
Restate-->>API: { pipelineId, invocationId }
API-->>Web: { pipelineId, invocationId }
end
rect rgb(240,220,255)
Note over Client,API: Poll status
Web->>API: GET /file-transcription/status/{pipelineId} with Bearer
API->>Restate: getStatus(pipelineId)
Restate-->>API: { status, transcript?, llmResult?, error? }
API-->>Web: StatusResponse
end
rect rgb(245,230,215)
Note over Client,API: Fetch final result
Web->>API: GET /file-transcription/result/{pipelineId} with Bearer
API->>Restate: attach & fetch final workflow result
Restate-->>API: final StatusResponse
API-->>Web: final StatusResponse
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/web/src/functions/billing.ts (1)
223-288: Extract shared customer setup logic to reduce duplication.The customer creation and persistence logic (lines 237-263) duplicates the same pattern from
createCheckoutSession(lines 73-99). Consider extracting this into a shared helper functionensureStripeCustomerto improve maintainability.Apply this diff to extract the shared logic:
+const ensureStripeCustomer = async ( + supabase: SupabaseClient, + user: AuthUser, + stripe: ReturnType<typeof getStripeClient>, +) => { + let stripeCustomerId = await getStripeCustomerIdForUser(supabase, { + id: user.id, + user_metadata: user.user_metadata, + }); + + if (!stripeCustomerId) { + const newCustomer = await stripe.customers.create({ + email: user.email, + metadata: { + userId: user.id, + }, + }); + + await Promise.all([ + supabase.auth.updateUser({ + data: { + stripe_customer_id: newCustomer.id, + }, + }), + supabase + .from("profiles") + .update({ stripe_customer_id: newCustomer.id }) + .eq("id", user.id), + ]); + + stripeCustomerId = newCustomer.id; + } + + return stripeCustomerId; +};Then simplify both functions:
export const createCheckoutSession = createServerFn({ method: "POST" }) .inputValidator(createCheckoutSessionInput) .handler(async ({ data }) => { // ... auth checks ... const stripe = getStripeClient(); - let stripeCustomerId = await getStripeCustomerIdForUser(supabase, { - id: user.id, - user_metadata: user.user_metadata, - }); - - if (!stripeCustomerId) { - const newCustomer = await stripe.customers.create({ - email: user.email, - metadata: { - userId: user.id, - }, - }); - - await Promise.all([ - supabase.auth.updateUser({ - data: { - stripe_customer_id: newCustomer.id, - }, - }), - supabase - .from("profiles") - .update({ stripe_customer_id: newCustomer.id }) - .eq("id", user.id), - ]); - - stripeCustomerId = newCustomer.id; - } + const stripeCustomerId = await ensureStripeCustomer(supabase, user, stripe); // ... rest of checkout logic ... });apps/api/src/routes/rpc.ts (1)
35-37: Consider logging errors for better observability.Returning
{ canStartTrial: false }on error implements a secure fail-closed pattern for this permission check. However, suppressing the error silently makes debugging database or RPC issues harder.Consider adding server-side logging:
if (error) { + console.error("can_start_trial RPC error:", error); return c.json({ canStartTrial: false }); }apps/api/openapi.gen.json (1)
294-427: Consider consolidating duplicate schemas for status and result endpoints.The
/file-transcription/status/{pipelineId}and/file-transcription/result/{pipelineId}endpoints return identical response schemas (lines 302-329 and 370-397). If these endpoints serve different purposes, consider:
- Documenting why both endpoints exist in the API descriptions
- Using a shared schema definition in components.schemas to reduce duplication
apps/web/src/functions/transcription.ts (1)
45-50: Consider extracting the API client creation into a helper function.The same client creation pattern with authorization header is repeated in all three pipeline functions. Extract it to reduce duplication.
// Helper at module level async function getAuthorizedApiClient() { const supabase = getSupabaseServerClient(); const { data: sessionData } = await supabase.auth.getSession(); if (!sessionData.session) { return null; } return createClient({ baseUrl: env.VITE_API_URL, headers: { Authorization: `Bearer ${sessionData.session.access_token}`, }, }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/api-client/src/generated/sdk.gen.tsis excluded by!**/generated/**,!**/generated/**,!**/*.gen.tspackages/api-client/src/generated/types.gen.tsis excluded by!**/generated/**,!**/generated/**,!**/*.gen.tspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
apps/api/openapi.gen.json(4 hunks)apps/api/package.json(1 hunks)apps/api/src/env.ts(1 hunks)apps/api/src/routes/file-transcription.ts(1 hunks)apps/api/src/routes/index.ts(2 hunks)apps/api/src/routes/rpc.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/src/env.ts(0 hunks)apps/web/src/functions/billing.ts(2 hunks)apps/web/src/functions/transcription.ts(5 hunks)apps/web/src/routes/_view/app/account.tsx(5 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/env.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/api/src/routes/rpc.tsapps/api/src/env.tsapps/web/src/functions/billing.tsapps/api/src/routes/file-transcription.tsapps/api/src/routes/index.tsapps/web/src/functions/transcription.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/api/src/routes/rpc.tsapps/api/src/env.tsapps/web/src/functions/billing.tsapps/api/src/routes/file-transcription.tsapps/web/src/routes/_view/app/account.tsxapps/api/src/routes/index.tsapps/web/src/functions/transcription.ts
🧠 Learnings (3)
📚 Learning: 2025-11-30T05:42:35.099Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/restate/AGENTS.md:0-0
Timestamp: 2025-11-30T05:42:35.099Z
Learning: Applies to apps/restate/**/restate/.env.example : Use Infisical to export environment variables for the restate app in dev environment with secret overriding disabled, outputting to dotenv format at $REPO/apps/restate/.env
Applied to files:
apps/api/package.jsonapps/web/package.json
📚 Learning: 2025-11-30T05:42:35.099Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/restate/AGENTS.md:0-0
Timestamp: 2025-11-30T05:42:35.099Z
Learning: Applies to apps/restate/**/.env* : Environment configuration should use the dev environment with secret-overriding set to false
Applied to files:
apps/api/package.json
📚 Learning: 2025-11-24T16:32:24.348Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/desktop-e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:24.348Z
Learning: Applies to apps/desktop-e2e/**/*.{test,spec}.{js,ts,tsx} : Refer to `scripts/setup-desktop-e2e.sh` for end-to-end test environment setup if setup is missing
Applied to files:
apps/web/package.json
🧬 Code graph analysis (4)
apps/web/src/functions/billing.ts (5)
apps/web/src/functions/supabase.ts (1)
getSupabaseServerClient(20-36)apps/api/src/env.ts (1)
env(4-34)apps/web/src/env.ts (1)
env(7-41)apps/api/src/integration/stripe.ts (1)
stripe(5-7)apps/web/src/functions/stripe.ts (1)
getStripeClient(6-10)
apps/api/src/routes/file-transcription.ts (3)
apps/web/src/functions/transcription.ts (1)
StatusStateType(23-28)apps/api/src/env.ts (1)
env(4-34)apps/api/src/hono-bindings.ts (1)
AppBindings(7-17)
apps/web/src/routes/_view/app/account.tsx (1)
apps/web/src/functions/billing.ts (2)
canStartTrial(196-221)createTrialCheckoutSession(223-288)
apps/web/src/functions/transcription.ts (1)
apps/web/src/env.ts (1)
env(7-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: fmt
- GitHub Check: ci
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (14)
apps/web/src/functions/billing.ts (1)
265-286: Trial configuration correctly implements no-card-required trial.The configuration properly sets up a 14-day trial that cancels automatically if no payment method is provided, which is appropriate for a frictionless trial experience.
apps/web/package.json (1)
6-6: LGTM! Environment configuration aligns with the Restate migration.Removing the Restate environment file from the web app is consistent with moving the Restate integration to the API layer.
apps/api/src/routes/index.ts (1)
5-5: LGTM! Route registration follows established patterns.The new file-transcription route is properly imported and mounted following the same conventions as other routes in the application.
Also applies to: 19-19
apps/web/src/routes/_view/app/account.tsx (1)
66-119: LGTM! Trial flow implementation follows established patterns.The integration properly uses
useQueryanduseMutationfrom TanStack Query, handles loading states correctly, and follows the coding guidelines for form/mutation state management. The conditional rendering logic is clear and maintainable.apps/api/package.json (1)
14-14: No issues found. The version @restatedev/restate-sdk-clients@^1.9.1 is valid, represents the latest release on npm (published 2025-10-27), and has no reported security vulnerabilities.apps/api/src/routes/file-transcription.ts (4)
1-12: LGTM!Clean imports with appropriate separation between external libraries and internal modules. The namespace import for
clientsis reasonable given the usage pattern.
13-29: LGTM!The
PipelineStatusenum andStatusResponseSchemaare well-defined and align correctly with theStatusStateTypeexported fromapps/web/src/functions/transcription.ts(lines 22-27).
47-59: LGTM!Input and output schemas are properly defined with appropriate required/optional fields.
86-98: Good path validation and ownership check.The validation correctly prevents path traversal attacks by rejecting
.and..segments, and enforces that users can only access their own files by comparingownerIdtouserId.apps/web/src/functions/transcription.ts (5)
1-11: LGTM!Good use of aliasing
createClienttocreateDeepgramClientto avoid naming conflicts with the API client'screateClient.
15-28: LGTM!Type definitions correctly mirror the API schema. The union type approach works well here.
71-106: LGTM!The function correctly handles authentication and API calls. The duplication was noted above as an optional refactor.
108-143: LGTM!Consistent implementation with proper error handling.
161-161: LGTM!Correctly uses the aliased
createDeepgramClientto maintain clarity.
No description provided.