feat(restate): add services folder, simplify audioPipeline, remove userRateLimiter#1997
feat(restate): add services folder, simplify audioPipeline, remove userRateLimiter#1997
Conversation
- Create apps/restate/src/services folder for organized service structure - Add rate-limit.ts: Virtual Object for per-key rate limiting with token bucket pattern - Add stt-file.ts: Workflow for STT file processing with Deepgram integration - Update index.ts to register new services alongside existing ones - Design supports future multiple LLM output versions Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ 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. |
📝 WalkthroughWalkthroughReplaces the per-user limiter with a new generic rate limiter and adds a new speech-to-text workflow that orchestrates Deepgram transcription, callback handling, and file cleanup. The audio pipeline is simplified to transcription-only (LLM steps removed). Index service registration updated accordingly; docs updated to reflect flow changes. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Restate as Restate App (workflow)
participant RateLimit as RateLimiter
participant Storage as File Storage
participant Deepgram as Deepgram API
Client->>Restate: Submit audio for transcription (run)
Restate->>RateLimit: checkAndConsume(userId)
alt Rate limit exceeded
RateLimit-->>Restate: TerminalError 429
Restate-->>Client: 429 Too Many Requests
else Within quota
RateLimit-->>Restate: OK
Restate->>Storage: Generate signed URL for file
Storage-->>Restate: Signed URL
Restate->>Deepgram: Start transcription (audioUrl, callbackUrl)
Deepgram-->>Restate: requestId
Restate->>Restate: await transcript (ctx.promise "transcript")
Deepgram->>Restate: Callback -> onTranscript(transcript)
Restate->>Restate: resolve transcript, set DONE
Restate->>Storage: Delete audio file (cleanup)
Storage-->>Restate: Deleted
Restate-->>Client: Success (transcript stored)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/restate/src/services/stt-file.ts (3)
81-81: Unsafe type assertion for environment access.The
as Envassertion assumesextraArgs[0]is always a validEnvobject. If the endpoint is misconfigured or the workflow is invoked incorrectly, this will fail silently and cause cryptic runtime errors when accessingenvproperties.Consider validating with
envSchema.parse()for consistency with how it's done inindex.ts, or at minimum add a runtime check.- const env = ctx.request().extraArgs[0] as Env; + const env = envSchema.parse(ctx.request().extraArgs[0]);This requires importing
envSchemafrom../env.
112-116: Silently swallowing cleanup errors may hide issues.The
.catch(() => {})pattern suppresses all cleanup failures. While the intent is reasonable (don't fail the workflow due to cleanup), completely ignoring errors makes debugging difficult if files aren't being deleted.Consider logging the error for observability:
await ctx.run("cleanup", () => - deleteFile(env, input.fileId).catch(() => {}), + deleteFile(env, input.fileId).catch((e) => { + console.warn(`Failed to cleanup file ${input.fileId}:`, e); + }), );
145-149: Minor: Redundant nullish coalescing.
transcript ?? undefinedanderror ?? undefinedare redundant since if the values are alreadyundefined, this expression returnsundefinedanyway. Thectx.get<string>()returnsstring | nulltypically, so if it returnsnull, the?? undefinedconverts it.This is fine as-is for explicit null-to-undefined conversion, but could be simplified if
ctx.getalready returnsundefinedfor missing keys.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/restate/src/index.ts(1 hunks)apps/restate/src/services/rate-limit.ts(1 hunks)apps/restate/src/services/stt-file.ts(1 hunks)
🧰 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/restate/src/services/rate-limit.tsapps/restate/src/services/stt-file.tsapps/restate/src/index.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/restate/src/services/rate-limit.tsapps/restate/src/services/stt-file.tsapps/restate/src/index.ts
🧬 Code graph analysis (1)
apps/restate/src/index.ts (3)
apps/restate/src/env.ts (2)
Env(12-12)envSchema(3-10)apps/restate/src/services/rate-limit.ts (1)
rateLimiter(13-43)apps/restate/src/services/stt-file.ts (1)
sttFile(69-153)
⏰ 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). (4)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (11)
apps/restate/src/index.ts (2)
5-6: LGTM!The new imports follow the existing import pattern and are correctly placed alphabetically within the services group.
13-13: LGTM!The services array correctly registers both new services alongside the existing ones. The endpoint will now expose all four services.
apps/restate/src/services/rate-limit.ts (4)
3-11: LGTM!The internal interfaces are well-defined and appropriately scoped (not exported), keeping the public API minimal.
39-41: LGTM!The reset handler correctly clears state, allowing for manual rate limit resets when needed.
45-49: LGTM!The
limiterhelper provides a clean, typed API for other services to acquire rate limiter clients by key. This is well-utilized bystt-file.ts.
16-37: I'll verify the concern about potential naming collision between the RateLimiter service and the existing userRateLimiter. Let me investigate the Restate service names and how they're registered.
<function_calls>
#!/bin/bashCheck the service name in rate-limit.ts
echo "=== Checking rate-limit.ts for service name ==="
rg -n "RateLimiter" apps/restate/src/services/rate-limit.ts -B2 -A2Check if there's a registration or export statement
echo -e "\n=== Checking for service registration in rate-limit.ts ==="
rg -n "export|restate." apps/restate/src/services/rate-limit.tsCheck the userRateLimiter file
echo -e "\n=== Checking userRateLimiter.ts ==="
cat apps/restate/src/userRateLimiter.tsCheck index.ts imports
echo -e "\n=== Checking index.ts ==="
cat apps/restate/src/index.ts
</function_calls>apps/restate/src/services/stt-file.ts (5)
1-8: LGTM!Imports are well-organized and all dependencies are appropriately sourced from their respective packages.
10-48: LGTM!The Zod schemas are well-structured. The
DeepgramCallbackschema correctly handles the nested optional structure of Deepgram's response format. Type exports are appropriately derived usingz.infer.
50-67: LGTM!The
transcribehelper cleanly encapsulates the Deepgram API call with proper error handling for both API errors and missing request IDs.
138-151: LGTM!The
getStatushandler correctly retrieves and returns the current workflow state with appropriate fallback defaults.
120-136: The promise resolution is inherently idempotent per Restate's durable-promise semantics—no issue exists.Restate's durable promises are single-shot primitives that can be resolved at most once. The outcome is recorded in workflow history immediately, and subsequent resolution attempts are benign no-ops. The existing check
if (existing !== undefined) return;provides an extra defensive layer, but even without it, concurrent callbacks cannot cause problems: Restate's design eliminates the race condition by guaranteeing a single canonical outcome.
- Simplify audioPipeline.ts: remove LLM processing, use new rate-limit service - Remove userRateLimiter.ts (replaced by services/rate-limit.ts) - Update index.ts to remove old userRateLimiter registration - Update MDX article to reference new RateLimiter and simplified flow Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
devin-ai-integration
bot
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/restate/src/audioPipeline.ts (2)
83-83: Unsafe type assertion for environment access.The
ctx.request().extraArgs[0] as Envpattern assumesextraArgsis populated and the first element is anEnvobject. If the deployment misconfigures this, you'll get a runtime error with no clear indication of the cause.Consider adding a validation check:
- const env = ctx.request().extraArgs[0] as Env; + const env = ctx.request().extraArgs[0] as Env | undefined; + if (!env?.DEEPGRAM_API_KEY || !env?.RESTATE_INGRESS_URL) { + throw new Error("Missing required environment configuration"); + }
115-117: Silent error swallowing in cleanup may hinder debugging.Swallowing cleanup errors with
.catch(() => {})is acceptable for non-fatal operations, but completely hiding failures makes it difficult to diagnose issues like permission errors or misconfigured storage.Consider logging the error:
await ctx.run("cleanup", () => - deleteFile(env, req.fileId).catch(() => {}), + deleteFile(env, req.fileId).catch((e) => { + console.warn(`Cleanup failed for fileId=${req.fileId}:`, e); + }), );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/restate/src/audioPipeline.ts(4 hunks)apps/restate/src/index.ts(1 hunks)apps/restate/src/userRateLimiter.ts(0 hunks)apps/web/content/articles/durable-meeting-summary.mdx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/restate/src/userRateLimiter.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/restate/src/index.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/restate/src/audioPipeline.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/restate/src/audioPipeline.ts
🧬 Code graph analysis (1)
apps/restate/src/audioPipeline.ts (2)
apps/restate/src/services/rate-limit.ts (1)
limiter(47-49)apps/restate/src/supabase.ts (2)
createSignedUrl(23-58)deleteFile(60-81)
⏰ 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: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (7)
apps/web/content/articles/durable-meeting-summary.mdx (2)
15-52: Architecture diagram accurately reflects new service changes.The mermaid diagram successfully captures the updated flow:
- Line 20: RateLimiter participant correctly represents the new generic rate-limiting service
- Lines 29-36: Rate-limit check flow with error/success paths is clear
- Line 50: Response payload correctly shows only
{ status: DONE, transcript }(LLM steps removed)This aligns well with the new services introduced in this PR (rate-limit.ts and stt-file.ts).
1-8: Document is unpublished with incomplete sections—clarify publication timeline.The article is marked
published: falseand contains multiple empty section headers (TL;DR, The Problem, Pipeline States, Deepgram Callback Pattern, Error Handling). Clarify whether this is a placeholder for a follow-up PR or if content should be added before merging.If this article is meant to ship with this PR, consider either:
- Adding placeholder text to each section to signal WIP, or
- Merging this as unpublished and opening a follow-up issue to complete the sections.
apps/restate/src/audioPipeline.ts (5)
1-50: Schema definitions look well-structured.The
DeepgramCallbackschema correctly handles both the batch (results.channels[].alternatives[]) and streaming (channel.alternatives[]) callback formats from Deepgram. The optional chaining in the schema aligns with the extraction logic inonDeepgramResult.
52-69: Clean transcription helper function.The function correctly uses Deepgram's callback-based transcription API and provides clear error messages prefixed with "Deepgram:" for easier debugging.
86-89: Rate limiting integration looks correct.The
limiter(ctx, req.userId)pattern correctly uses the new rate-limit service to enforce per-user throttling. The hardcoded values (60s window, 5 max) align with the PR's stated design.
122-138: Good idempotency handling in callback.The early return when
existing !== undefinedcorrectly prevents duplicate processing if the callback is delivered multiple times. The transcript extraction handles both Deepgram response formats with appropriate fallback to empty string.
140-154: Status handler implementation is correct.The handler properly retrieves all relevant state and returns a consistent
StatusStateTypeobject with appropriate defaults.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
1 participant
Summary
Adds a new
services/folder structure underapps/restate/src/and consolidates the codebase:New services:
limiter()helper for easy consumption from other services.Simplifications:
services/rate-limit.ts, simplified status states from 6 to 4 (QUEUED,TRANSCRIBING,DONE,ERROR)services/rate-limit.ts[audioPipeline, rateLimiter, sttFile]Review & Testing Checklist for Human
apps/web/src/functions/transcription.tsexpectsStatusStateTypewithllmResultand statusesTRANSCRIBED/LLM_RUNNINGwhich are now removed. The UI only usesstatusandtranscript, but the Zod schemas may need updating to match.AudioPipeline(/AudioPipeline/{key}/onDeepgramResult) andSttFile(/SttFile/{key}/onTranscript) have callback handlers - verify they work with actual Deepgram webhooks.ctx.request().extraArgs[0] as Envtype assertion could fail at runtime if env isn't properly passed through the endpoint handler.UserRateLimiterservice name is gone - any external systems calling it will break.Recommended test plan: Deploy to a staging Restate environment and trigger both
AudioPipeline/runandSttFile/runinvocations with valid audio files. Verify the web file-transcription page still works end-to-end.Notes
sttFileworkflow is designed to be composable for future multiple LLM output versions (as mentioned in requirements) - it only handles STT, leaving LLM processing to be added separately.durable-meeting-summary.mdxarticle to reflect simplified flow (removed LLM step, renamed RateLimiter).Link to Devin run: https://app.devin.ai/sessions/2dd61f559b824d26ae2c2f8129ff01e4
Requested by: yujonglee (@yujonglee)