feat(admin): add durably for resumable background translation jobs#118
feat(admin): add durably for resumable background translation jobs#118
Conversation
- Add @coji/durably and @coji/durably-react packages - Create durably server instance with translation job definition - Add durably client for type-safe React hooks - Create API endpoint for durably handler - Update project index to use useJob hook with progress display - Show translated file count progress during translation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@coji has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a Durably-based "translate-project" job, client hooks and UI for starting and tracking translation runs, a durably HTTP route, exports for durably client/types/handler, and related dependency and Vite plugin changes. Changes
Sequence DiagramsequenceDiagram
participant User as User / Browser
participant Route as Project Route (client)
participant DurablyClient as Durably Client
participant API as /api/durably Handler
participant Service as Translation Job Service
participant DB as Database
User->>Route: Click "Start Translation"
Route->>DurablyClient: run "translate-project" with {projectId}
DurablyClient->>API: HTTP request to /api/durably
API->>Service: durablyHandler dispatches to job
Service->>DB: Fetch project and updated files
DB-->>Service: Return files list
loop For each file
Service->>Service: translateByGemini(file)
Service->>DB: Update file record (is_updated, translation, timestamps)
Service->>DurablyClient: Report progress
DurablyClient-->>Route: useRuns updates with progress
Route->>Route: Update Translation Progress UI
end
Service-->>DurablyClient: Return final stats
DurablyClient-->>Route: useRuns reports completion
Route->>Route: Add run to Translation History Sheet
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Claude finished @coji's task —— View job Code Review CompleteI've thoroughly reviewed the PR implementation for adding durably workflow engine for resumable background translation jobs. Here's my comprehensive feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
Change api.durably.ts to api.durably.$.ts to match nested paths like /api/durably/trigger and /api/durably/status Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @coji's task —— View job Code Review CompleteI've thoroughly reviewed the PR implementation for adding durably workflow engine for resumable background translation jobs. Here's my comprehensive feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
![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 (2)
apps/admin/app/services/durably.server.ts (1)
31-47: Remove unusedprojectvariable from destructuring.The
fetch-datastep returns{ project, files }, but onlyfilesis destructured and used. Either removeprojectfrom the return object or use it (e.g., for logging the project name).♻️ Suggested fix
- const { files } = await step.run('fetch-data', async () => { - const project = await db + const { project, files } = await step.run('fetch-data', async () => { + const projectRecord = await db .selectFrom('projects') .selectAll() .where('id', '=', projectId) .executeTakeFirstOrThrow() const files = await db .selectFrom('files') .selectAll() .where('project_id', '=', projectId) .where('is_updated', '=', 1) .orderBy('created_at', 'asc') .execute() - return { project, files } + return { project: projectRecord, files } }) - step.log.info(`Starting translation for project: ${projectId}`) + step.log.info(`Starting translation for project: ${project.id} (${project.name ?? projectId})`)Alternatively, if the project data isn't needed:
♻️ Remove unused query
const { files } = await step.run('fetch-data', async () => { - const project = await db - .selectFrom('projects') - .selectAll() - .where('id', '=', projectId) - .executeTakeFirstOrThrow() - const files = await db .selectFrom('files') .selectAll() .where('project_id', '=', projectId) .where('is_updated', '=', 1) .orderBy('created_at', 'asc') .execute() - return { project, files } + return { files } })apps/admin/app/routes/_app+/$project._index/route.tsx (1)
201-208: Avoid inline type assertions; use proper typing from job output.The type assertions
as { translatedCount: number }andas { errorCount: number }are fragile and bypass TypeScript's type safety. SincedurablyClientis typed withtypeof jobs, theoutputshould already be typed according to the job's output schema.♻️ Suggested improvement
If the hook doesn't provide proper output typing, consider extracting the output type:
// At the top of the component or in a types file type TranslationOutput = { translatedCount: number errorCount: number totalCount: number } // Then in the JSX {translationJob.output && ( <div className="text-sm text-muted-foreground"> Completed:{' '} {(translationJob.output as TranslationOutput).translatedCount}{' '} translated,{' '} {(translationJob.output as TranslationOutput).errorCount} errors </div> )}Or ideally, if the library supports it, the output should be inferred from the job definition's output schema.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/admin/app/routes/_app+/$project._index/route.tsxapps/admin/app/routes/api.durably.tsapps/admin/app/services/durably.client.tsapps/admin/app/services/durably.server.tsapps/admin/package.json
🧰 Additional context used
📓 Path-based instructions (2)
apps/admin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Kysely ORM with SQLite for database access in the admin app
Files:
apps/admin/app/routes/api.durably.tsapps/admin/app/services/durably.client.tsapps/admin/app/services/durably.server.tsapps/admin/app/routes/_app+/$project._index/route.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for type checking across all applications
Files:
apps/admin/app/routes/api.durably.tsapps/admin/app/services/durably.client.tsapps/admin/app/services/durably.server.tsapps/admin/app/routes/_app+/$project._index/route.tsx
🧬 Code graph analysis (2)
apps/admin/app/services/durably.server.ts (3)
apps/admin/app/services/db.types.ts (1)
Database(10-15)apps/admin/app/services/db.server.ts (1)
db(21-30)apps/admin/app/services/translate-gemini.ts (1)
translateByGemini(143-195)
apps/admin/app/routes/_app+/$project._index/route.tsx (2)
apps/admin/app/services/durably.client.ts (1)
durablyClient(4-6)apps/admin/app/components/ui/progress.tsx (1)
Progress(29-29)
⏰ 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). (1)
- GitHub Check: claude-review
🔇 Additional comments (6)
apps/admin/app/services/durably.client.ts (1)
1-6: LGTM!Clean and minimal client setup with proper type inference from the server-side job definitions. The API path
/api/durablycorrectly matches the route handler.apps/admin/app/services/durably.server.ts (1)
56-95: Good resumability design with per-file steps.Each file translation is wrapped in its own
step.run, enabling job resumption from the last successful file. The error handling gracefully continues to the next file while tracking failures.apps/admin/app/routes/_app+/$project._index/route.tsx (2)
97-104: LGTM - Clean hook integration.The Durably client hook usage is straightforward and properly tracks the running state for UI feedback.
138-147: LGTM - Proper button behavior.Using
type="button"correctly prevents form submission, and theonClickhandler properly triggers the Durably job. The disabled state covers both form submission and running job scenarios.apps/admin/app/routes/api.durably.ts (1)
4-10: Verify authentication requirements for this endpoint.The route delegates directly to
durablyHandlerwithout visible authentication checks. Ensure that upstream middleware or route configuration provides adequate access control, especially since this endpoint can trigger and monitor background jobs. If auth is handled by parent route configuration, durablyHandler itself, or middleware, please confirm and document this design assumption.apps/admin/package.json (1)
21-22: The specified package versions do not exist on the npm registry.The packages
@coji/durably@0.7.0and@coji/durably-react@0.7.0are not found on the public npm registry. This will cause installation to fail. Either these are private packages requiring special registry configuration, or the versions need to be corrected. Additionally, if these should be added to the pnpm workspace catalog, use thecatalog:prefix pattern consistent with other dependencies.⛔ Skipped due to learnings
Learnt from: coji Repo: coji/remix-docs-ja PR: 104 File: pnpm-workspace.yaml:6-6 Timestamp: 2025-07-17T06:03:32.632Z Learning: In pnpm monorepos using the catalog feature, individual package.json files should reference dependencies with "catalog:" instead of specific version numbers. The actual versions are defined in the catalog section of pnpm-workspace.yaml, providing centralized dependency management across the workspace. This is the correct and intended usage pattern.
- Import useJob from @coji/durably-react/client directly - Remove unnecessary durably.client.ts wrapper - Pass api and jobName to useJob for server-connected mode Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @coji's task —— View job Code Review CompleteI've thoroughly reviewed the PR implementation for adding durably workflow engine for resumable background translation jobs. Here's my comprehensive feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
- Add History button with Sheet to view past translation jobs - Show running job progress with spinner animation - Display error details in tooltip when hovering error count - Store translation errors in job output for later review - Share Kysely dialect between app and durably - Add vite-plugin-devtools-json for Chrome DevTools integration - Auto-follow latest running job on page load Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @coji's task —— View job I've completed my comprehensive code review of this PR. Here's my detailed feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
- Add type definitions for TranslationJobOutput and JobProgress - Remove unused project variable, keep only validation - Limit stored errors to MAX_STORED_ERRORS (50) to prevent huge output - Separate errorCount from errors.length for accurate counting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @apps/admin/app/services/durably.server.ts:
- Around line 55-65: The update uses now() twice causing tiny timestamp skew
between translated_at and updated_at; capture a single timestamp before the
update (e.g., const ts = new Date() or a single now()/sql literal) and set both
translated_at and updated_at to that same ts in the updateTable('files') .set
call so both columns receive identical timestamps for the row update.
- Around line 98-114: Top-level await of durably.init() can run multiple times
during dev reloads; guard it by storing a global singleton promise and awaiting
that instead of calling durably.init() directly. Create or reuse a
process-global like globalThis.__durablyInitPromise (or similar unique name)
that is assigned to durably.init() if not already present, then await that
promise; keep the rest of the exports (durably, jobs, durablyHandler) intact and
ensure any TS typing uses any/casting for the global to avoid compile errors.
- Around line 7-95: The progress message in the translate-project job is
misleading: step.progress(i + 1, ...) shows all processed files but the text
"Translated X/Y" implies only successes; update the step.progress call inside
the for-loop in the run function to show accurate status (for example use
step.progress(i + 1, files.length, `Processed ${i + 1}/${files.length}
(translated ${translatedCount}, errors ${errors.length})`) or alternatively only
increment the first arg with translatedCount if you want the progress bar to
represent successful translations), making sure to reference the existing
translatedCount and errors.length variables when building the message.
In @apps/admin/package.json:
- Around line 20-23: Add the missing catalog entries before switching
package.json deps to catalog:—update the workspace catalog to include
'@coji/durably': 0.7.0, '@coji/durably-react': 0.7.0, and
'vite-plugin-devtools-json': 1.0.0 (in pnpm-workspace.yaml catalog block), then
change the entries in apps/admin package.json so the dependencies
"@coji/durably", "@coji/durably-react" and "vite-plugin-devtools-json" use the
"catalog:" spec like the other cataloged packages.
🧹 Nitpick comments (2)
apps/admin/vite.config.ts (1)
4-17: Consider enablingdevtoolsJson()only in dev mode.If this plugin emits extra artifacts/metadata, you probably don’t want it in production builds.
Proposed diff
import { reactRouter } from '@react-router/dev/vite' import tailwindcss from '@tailwindcss/vite' import { defineConfig } from 'vite' import devtoolsJson from 'vite-plugin-devtools-json' import tsconfigPaths from 'vite-tsconfig-paths' import { configDefaults } from 'vitest/config' -export default defineConfig({ +export default defineConfig(({ mode }) => ({ optimizeDeps: { exclude: ['projects'] }, plugins: [ - devtoolsJson(), + ...(mode === 'development' ? [devtoolsJson()] : []), tailwindcss(), reactRouter(), tsconfigPaths({ skip: (dir) => dir.includes('projects') || dir.includes('node_modules'), }), ], server: { port: 5170, }, test: { exclude: [...configDefaults.exclude, 'projects'] }, -}) +}))apps/admin/app/routes/_app+/$project._index/route.tsx (1)
183-325: History UI is solid; consider reducing repeated casts + TooltipProvider nesting.Not blocking, but extracting
const progress = run.progress as .../const output = run.output as ...would simplify and reduce copy/paste casts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/admin/app/routes/_app+/$project._index/route.tsxapps/admin/app/routes/api.durably.$.tsapps/admin/app/services/db.server.tsapps/admin/app/services/durably.server.tsapps/admin/package.jsonapps/admin/vite.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/admin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Kysely ORM with SQLite for database access in the admin app
Files:
apps/admin/app/services/db.server.tsapps/admin/vite.config.tsapps/admin/app/routes/api.durably.$.tsapps/admin/app/services/durably.server.tsapps/admin/app/routes/_app+/$project._index/route.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for type checking across all applications
Files:
apps/admin/app/services/db.server.tsapps/admin/vite.config.tsapps/admin/app/routes/api.durably.$.tsapps/admin/app/services/durably.server.tsapps/admin/app/routes/_app+/$project._index/route.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: coji/remix-docs-ja PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T07:30:36.365Z
Learning: Admin app uses Gemini API integration for translation management features
📚 Learning: 2026-01-11T07:30:36.365Z
Learnt from: CR
Repo: coji/remix-docs-ja PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T07:30:36.365Z
Learning: Applies to apps/admin/**/*.{ts,tsx} : Use Kysely ORM with SQLite for database access in the admin app
Applied to files:
apps/admin/app/services/db.server.ts
📚 Learning: 2026-01-11T07:30:36.365Z
Learnt from: CR
Repo: coji/remix-docs-ja PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T07:30:36.365Z
Learning: Applies to {vite.config.{ts,js},tailwind.config.{ts,js}} : Use Vite for bundling with Tailwind CSS v4 and Vite plugin integration
Applied to files:
apps/admin/vite.config.tsapps/admin/package.json
📚 Learning: 2026-01-11T07:30:36.365Z
Learnt from: CR
Repo: coji/remix-docs-ja PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T07:30:36.365Z
Learning: Applies to apps/*/react-router.config.ts : Use react-router.config.ts for routing configuration in each app
Applied to files:
apps/admin/vite.config.ts
📚 Learning: 2026-01-11T07:30:36.365Z
Learnt from: CR
Repo: coji/remix-docs-ja PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T07:30:36.365Z
Learning: Admin app uses Gemini API integration for translation management features
Applied to files:
apps/admin/app/services/durably.server.ts
📚 Learning: 2025-07-17T06:03:32.632Z
Learnt from: coji
Repo: coji/remix-docs-ja PR: 104
File: pnpm-workspace.yaml:6-6
Timestamp: 2025-07-17T06:03:32.632Z
Learning: In pnpm monorepos using the catalog feature, individual package.json files should reference dependencies with "catalog:" instead of specific version numbers. The actual versions are defined in the catalog section of pnpm-workspace.yaml, providing centralized dependency management across the workspace. This is the correct and intended usage pattern.
Applied to files:
apps/admin/package.json
🧬 Code graph analysis (2)
apps/admin/app/routes/api.durably.$.ts (1)
apps/admin/app/services/durably.server.ts (1)
durablyHandler(114-114)
apps/admin/app/services/durably.server.ts (2)
apps/admin/app/services/db.server.ts (2)
db(21-30)now(33-33)apps/admin/app/services/translate-gemini.ts (1)
translateByGemini(143-195)
⏰ 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). (1)
- GitHub Check: claude-review
🔇 Additional comments (3)
apps/admin/app/services/db.server.ts (1)
15-23: Exportingdialectis fine; ensure it remains server-only.Since this file imports
better-sqlite3, any accidental client import will break builds/runtime.apps/admin/app/routes/_app+/$project._index/route.tsx (2)
344-385: Consider checking for specific active job states instead of relying on truthiness alone.The condition
translationJob.status &&renders the block if status is any truthy value. Ifstatuscan be a string like'idle'or other non-active states, this may display the progress section unexpectedly. Look at the history section pattern (run.status === 'running' || run.status === 'pending') and consider whether an explicit check for active states (e.g.,['running', 'pending'].includes(translationJob.status)) would be more appropriate.
108-133: Make run selection deterministic and handletrigger()errors.
runs?.find(...)only works as "latest" ifrunsis already sorted by theuseRunshook.translationJob.trigger(...)should be awaited (or explicitlyvoided) with error handling if the library returns a Promise that can reject.The concerns cannot be definitively verified without access to the
@coji/durably-reactlibrary source code or documentation (the package is private and no public documentation is available). Consider reviewing the library's API contract directly or adding explicit error boundaries if the library documentation indicatestrigger()can throw.
| import { durablyHandler } from '~/services/durably.server' | ||
| import type { Route } from './+types/api.durably.$' | ||
|
|
||
| export async function loader({ request }: Route.LoaderArgs) { | ||
| return await durablyHandler.handle(request, '/api/durably') | ||
| } | ||
|
|
||
| export async function action({ request }: Route.ActionArgs) { | ||
| return await durablyHandler.handle(request, '/api/durably') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the durably.server file to understand the handler
find apps/admin -name "*durably*" -type fRepository: coji/remix-docs-ja
Length of output: 142
🏁 Script executed:
# Check the admin app structure and look for layout files that might enforce auth
git ls-files apps/admin/app/routes | head -20Repository: coji/remix-docs-ja
Length of output: 1261
🏁 Script executed:
# Search for how durablyHandler is defined and used
rg "durablyHandler" apps/admin --type ts --type tsx -B 2 -A 5Repository: coji/remix-docs-ja
Length of output: 89
🏁 Script executed:
# Search for durablyHandler without specifying file types
rg "durablyHandler" apps/admin -B 2 -A 5Repository: coji/remix-docs-ja
Length of output: 1039
🏁 Script executed:
# Read the durably.server.ts file to understand the handler implementation
cat -n apps/admin/app/services/durably.server.tsRepository: coji/remix-docs-ja
Length of output: 4116
🏁 Script executed:
# Look for layout files in admin app that might enforce authentication
cat -n apps/admin/app/routes/_app+/_layout.tsxRepository: coji/remix-docs-ja
Length of output: 744
🏁 Script executed:
# Check if there's a root layout in admin app
git ls-files apps/admin/app/routes | grep -E "layout|root"Repository: coji/remix-docs-ja
Length of output: 101
🏁 Script executed:
# Look for root layout or auth middleware in admin app
git ls-files apps/admin/app | grep -E "root|layout|middleware|auth"Repository: coji/remix-docs-ja
Length of output: 125
🏁 Script executed:
# Check the admin app root level for any layout or entry files
cat -n apps/admin/app/entry.server.tsxRepository: coji/remix-docs-ja
Length of output: 125
🏁 Script executed:
# Look for route guards or middleware patterns
rg "beforeEach|middleware|guard|protect|auth" apps/admin/app/routes -A 3Repository: coji/remix-docs-ja
Length of output: 44
🏁 Script executed:
# Check if there's a root layout that wraps the _app layout
git ls-files apps/admin/app/routes | grep -v "_app" | grep -E "route|layout" | head -10Repository: coji/remix-docs-ja
Length of output: 100
🏁 Script executed:
# Check the root.tsx for auth protection or middleware
cat -n apps/admin/app/root.tsxRepository: coji/remix-docs-ja
Length of output: 2148
🏁 Script executed:
# Look for any auth-related files in admin app
find apps/admin -name "*auth*" -o -name "*guard*" -o -name "*protect*"Repository: coji/remix-docs-ja
Length of output: 44
🏁 Script executed:
# Check how the _app layout is nested relative to api routes
git ls-files apps/admin/app/routes | sortRepository: coji/remix-docs-ja
Length of output: 1261
🏁 Script executed:
# Look for createDurablyHandler implementation to understand basePath param
rg "createDurablyHandler" --type ts -B 5 -A 10Repository: coji/remix-docs-ja
Length of output: 1346
🏁 Script executed:
# Check if there are any query/mutation files in the _app+ layout to understand the auth pattern
find apps/admin/app/routes/_app+ -name "*.server.ts" | head -5 | xargs catRepository: coji/remix-docs-ja
Length of output: 1987
🏁 Script executed:
# Look for environment variables or secrets related to durably auth
rg "DURABLY|durably" apps/admin/app --type ts -iRepository: coji/remix-docs-ja
Length of output: 1416
🏁 Script executed:
# Check if api.durably is explicitly referenced elsewhere or if there's any URL construction
rg "api/durably|/api/durably" apps/admin -iRepository: coji/remix-docs-ja
Length of output: 421
🏁 Script executed:
# Look at the package.json to understand @coji/durably version and see if there's documentation
cat -n apps/admin/package.json | grep -A 5 -B 5 durablyRepository: coji/remix-docs-ja
Length of output: 563
🏁 Script executed:
# Check the _app+ layout more carefully and look for any middleware or data loading
cat -n apps/admin/app/routes/_app+/_layout.tsxRepository: coji/remix-docs-ja
Length of output: 744
🏁 Script executed:
# Search for any server middleware or authentication enforcement
find apps/admin -name "*.server.*" | xargs rg "auth|middleware|guard" -lRepository: coji/remix-docs-ja
Length of output: 46
🏁 Script executed:
# Look at how the client uses durably to understand the api endpoint better
cat -n apps/admin/app/routes/_app+/$project._index/route.tsx | grep -A 10 -B 10 "api/durably"Repository: coji/remix-docs-ja
Length of output: 139
🏁 Script executed:
# Check if there's any environment setup or server configuration
git ls-files apps/admin | grep -E "server|config|middleware|auth"Repository: coji/remix-docs-ja
Length of output: 894
🏁 Script executed:
# Find the correct path for the project index route
find apps/admin/app/routes -name "*project*" -name "*index*"Repository: coji/remix-docs-ja
Length of output: 155
🏁 Script executed:
# Look at the project index route
cat -n apps/admin/app/routes/_app+/$project._index/route.tsx | head -80Repository: coji/remix-docs-ja
Length of output: 139
🏁 Script executed:
# Check if there's a loader or action in the project index route that might check auth
rg "loader|action" apps/admin/app/routes/_app+/$project._index/route.tsx -A 10Repository: coji/remix-docs-ja
Length of output: 147
🏁 Script executed:
# Look at the root loader to see if there's any global auth setup
rg "loader" apps/admin/app/root.tsx -A 15Repository: coji/remix-docs-ja
Length of output: 1008
🏁 Script executed:
# Read the project index route correctly
cat -n "apps/admin/app/routes/_app+/\$project._index/route.tsx" | head -100Repository: coji/remix-docs-ja
Length of output: 3303
🏁 Script executed:
# Search for where the durably api is configured on the client
rg "api.*durably" apps/admin/app/routes/_app+ -B 5 -A 5Repository: coji/remix-docs-ja
Length of output: 1828
🏁 Script executed:
# Look for the actual project index route using find
find apps/admin/app/routes/_app+ -type d -name "*project*"Repository: coji/remix-docs-ja
Length of output: 301
🏁 Script executed:
# Check the directory contents directly
ls -la "apps/admin/app/routes/_app+/" 2>/dev/null | head -20Repository: coji/remix-docs-ja
Length of output: 677
🏁 Script executed:
# Check the route structure more carefully - is api.durably at the same level as _app+ or nested?
git ls-files apps/admin/app/routes | grep -E "^apps/admin/app/routes/[^/]+$"Repository: coji/remix-docs-ja
Length of output: 100
🏁 Script executed:
# Confirm the api.durably.$.ts file location
ls -la apps/admin/app/routes/ | grep durablyRepository: coji/remix-docs-ja
Length of output: 126
🏁 Script executed:
# Look at the actual file structure to understand nesting
find apps/admin/app/routes -maxdepth 1 -type f -o -type d | sortRepository: coji/remix-docs-ja
Length of output: 150
🏁 Script executed:
# Check if there's any global middleware or auth setup that would protect ALL routes
rg "middleware|beforeEach|guard" apps/admin/app -B 2 -A 5Repository: coji/remix-docs-ja
Length of output: 44
🏁 Script executed:
# Check if there's a server configuration or middleware setup in react-router config
cat -n apps/admin/react-router.config.tsRepository: coji/remix-docs-ja
Length of output: 201
🏁 Script executed:
# Look for any server middleware or guards in vite config
cat -n apps/admin/vite.config.tsRepository: coji/remix-docs-ja
Length of output: 851
🏁 Script executed:
# Search for any auth patterns or middleware in the entire admin app
rg "middleware|createMiddleware|auth" apps/admin --type ts | head -20Repository: coji/remix-docs-ja
Length of output: 590
🏁 Script executed:
# Check if there's environment-based protection or any README documenting auth
find apps/admin -name "README*" -o -name "*.md" | xargs cat 2>/dev/null | head -50Repository: coji/remix-docs-ja
Length of output: 1763
🏁 Script executed:
# Look at the basePath parameter in @coji/durably context - check if it's used for routing validation
rg "basePath" apps/admin -B 3 -A 3Repository: coji/remix-docs-ja
Length of output: 44
Add authentication checks to /api/durably/* endpoint to prevent unauthorized job operations.
The route is accessible without authentication. Any caller can trigger or query translation jobs unless protection is enforced at the handler or request level. This is a real risk if the admin app is exposed beyond internal use.
The basePath parameter is correct for splat routing.
Optional refactoring (eliminate code duplication)
import { durablyHandler } from '~/services/durably.server'
import type { Route } from './+types/api.durably.$'
+const handle = (request: Request) => durablyHandler.handle(request, '/api/durably')
+
export async function loader({ request }: Route.LoaderArgs) {
- return await durablyHandler.handle(request, '/api/durably')
+ return handle(request)
}
export async function action({ request }: Route.ActionArgs) {
- return await durablyHandler.handle(request, '/api/durably')
+ return handle(request)
}📝 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.
| import { durablyHandler } from '~/services/durably.server' | |
| import type { Route } from './+types/api.durably.$' | |
| export async function loader({ request }: Route.LoaderArgs) { | |
| return await durablyHandler.handle(request, '/api/durably') | |
| } | |
| export async function action({ request }: Route.ActionArgs) { | |
| return await durablyHandler.handle(request, '/api/durably') | |
| } | |
| import { durablyHandler } from '~/services/durably.server' | |
| import type { Route } from './+types/api.durably.$' | |
| const handle = (request: Request) => durablyHandler.handle(request, '/api/durably') | |
| export async function loader({ request }: Route.LoaderArgs) { | |
| return handle(request) | |
| } | |
| export async function action({ request }: Route.ActionArgs) { | |
| return handle(request) | |
| } |
| export const durably = createDurably({ | ||
| dialect, | ||
| pollingInterval: 1000, | ||
| heartbeatInterval: 5000, | ||
| staleThreshold: 30000, | ||
| }).register({ | ||
| 'translate-project': translationJob, | ||
| }) | ||
|
|
||
| // Initialize durably tables | ||
| await durably.init() | ||
|
|
||
| // Export jobs for type inference on client | ||
| export const jobs = durably.jobs | ||
|
|
||
| // Create handler for API routes | ||
| export const durablyHandler = createDurablyHandler(durably) |
There was a problem hiding this comment.
Guard await durably.init() to avoid repeated init during dev reloads / multi-import.
Module-scope top-level init can run multiple times in some server setups; better to ensure it’s executed once per process.
One possible diff (global singleton promise)
export const durably = createDurably({
dialect,
pollingInterval: 1000,
heartbeatInterval: 5000,
staleThreshold: 30000,
}).register({
'translate-project': translationJob,
})
-// Initialize durably tables
-await durably.init()
+// Initialize durably tables (once per process)
+const globalForDurably = globalThis as unknown as {
+ __durablyInitPromise?: Promise<void>
+}
+globalForDurably.__durablyInitPromise ??= durably.init()
+await globalForDurably.__durablyInitPromise
// Export jobs for type inference on client
export const jobs = durably.jobs📝 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.
| export const durably = createDurably({ | |
| dialect, | |
| pollingInterval: 1000, | |
| heartbeatInterval: 5000, | |
| staleThreshold: 30000, | |
| }).register({ | |
| 'translate-project': translationJob, | |
| }) | |
| // Initialize durably tables | |
| await durably.init() | |
| // Export jobs for type inference on client | |
| export const jobs = durably.jobs | |
| // Create handler for API routes | |
| export const durablyHandler = createDurablyHandler(durably) | |
| export const durably = createDurably({ | |
| dialect, | |
| pollingInterval: 1000, | |
| heartbeatInterval: 5000, | |
| staleThreshold: 30000, | |
| }).register({ | |
| 'translate-project': translationJob, | |
| }) | |
| // Initialize durably tables (once per process) | |
| const globalForDurably = globalThis as unknown as { | |
| __durablyInitPromise?: Promise<void> | |
| } | |
| globalForDurably.__durablyInitPromise ??= durably.init() | |
| await globalForDurably.__durablyInitPromise | |
| // Export jobs for type inference on client | |
| export const jobs = durably.jobs | |
| // Create handler for API routes | |
| export const durablyHandler = createDurablyHandler(durably) |
🤖 Prompt for AI Agents
In @apps/admin/app/services/durably.server.ts around lines 98 - 114, Top-level
await of durably.init() can run multiple times during dev reloads; guard it by
storing a global singleton promise and awaiting that instead of calling
durably.init() directly. Create or reuse a process-global like
globalThis.__durablyInitPromise (or similar unique name) that is assigned to
durably.init() if not already present, then await that promise; keep the rest of
the exports (durably, jobs, durablyHandler) intact and ensure any TS typing uses
any/casting for the global to avoid compile errors.
|
Claude finished @coji's task —— View job I've completed my comprehensive code review of this PR. Here's my detailed feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/admin/app/routes/_app+/$project._index/route.tsx (3)
41-53: Avoidas JobProgress/as TranslationJobOutputcasts without runtime validation.
run.progress,run.output, andtranslationJob.outputare treated as trusted shapes; if the server changes or returns partial data, this can crash rendering (e.g.,output.translatedCountundefined) or show nonsense. Consider validating via Zod once before use.Sketch of a safer approach (local Zod schemas)
+const JobProgressSchema = z.object({ + current: z.number(), + total: z.number(), + message: z.string().optional(), +}) + +const TranslationJobOutputSchema = z.object({ + translatedCount: z.number(), + errorCount: z.number(), + totalCount: z.number(), + errors: z.array(z.object({ path: z.string(), error: z.string() })), +})Then replace casts with
safeParse(...)and render only when valid.Also applies to: 246-305, 353-392
197-333: ReduceTooltipProviderduplication (wrap once).
You create a newTooltipProviderfor each tooltip instance; typically you only need one provider around the sheet content.
124-128: Extract repeated durably config + verify non-standard width class.
api: '/api/durably'andjobName: 'translate-project'are repeated; consider constants to prevent drift. AlsoclassName="w-125 sm:max-w-125"looks like a non-default Tailwind scale—confirm it’s defined (or usew-[...]).Also applies to: 136-140, 204-205
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/admin/app/routes/_app+/$project._index/route.tsxapps/admin/app/services/durably.server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/admin/app/services/durably.server.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/admin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Kysely ORM with SQLite for database access in the admin app
Files:
apps/admin/app/routes/_app+/$project._index/route.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for type checking across all applications
Files:
apps/admin/app/routes/_app+/$project._index/route.tsx
⏰ 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). (1)
- GitHub Check: claude-review
🔇 Additional comments (2)
apps/admin/app/routes/_app+/$project._index/route.tsx (2)
142-145: [Rewritten review comment]
[Classification tag]
122-140: Consider explicitly selecting the latest running job bycreatedAtrather than relying on array ordering.The
runs?.find()approach assumes theuseRunsAPI returns runs ordered newest-first, but this guarantee is not documented. If multiple jobs with status'running'or'pending'exist (possible withpageSize: 20),find()may select the wrong one. Explicitly comparecreatedAttimestamps to ensure you always follow the most recent job.Suggested fix
- // Find the latest running job to follow - const latestRunningJob = runs?.find( - (run) => run.status === 'running' || run.status === 'pending', - ) + // Find the latest running job to follow (explicitly by createdAt) + const latestRunningJob = runs + ?.filter((run) => run.status === 'running' || run.status === 'pending') + ?.reduce<(typeof runs)[number] | undefined>((latest, run) => { + if (!latest) return run + return dayjs(run.createdAt).isAfter(dayjs(latest.createdAt)) ? run : latest + }, undefined)
- Use single timestamp variable to avoid skew between translated_at and updated_at - Change progress message from "Translated" to "Processed" with breakdown Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @coji's task —— View job Code Review CompleteI've thoroughly reviewed the PR implementation for adding durably workflow engine for resumable background translation jobs. Here's my comprehensive feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
CodeRabbit レビュー対応レビューコメントを評価し、対応しました。 ✅ 対応済み
❌ 対応不要と判断
|
…r dependency management
|
Claude finished @coji's task —— View job Code Review CompleteI've thoroughly reviewed the PR implementation for adding durably workflow engine for resumable background translation jobs. Here's my comprehensive feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
- Remove manual workaround using useRuns + initialRunId - useJob now automatically resumes and follows latest job by default Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @coji's task —— View job Code Review CompleteI've thoroughly reviewed the PR implementation for adding durably workflow engine for resumable background translation jobs. Here's my comprehensive feedback: Todo List
🎯 Overall AssessmentThis is a well-structured implementation that successfully integrates the durably workflow engine. The code follows good patterns and the architecture is clean. ✅ Strengths
🔍 Code Quality IssuesMinor Issues
Performance Considerations
🔒 Security Review✅ No security issues identified
🧪 Test Coverage Concerns❌ Missing Test Coverage
🎯 Specific Recommendations1. Add Error Recovery// In durably.server.ts, consider adding retry logic
const result = await step.run(`translate-file-${file.id}`, async () => {
// Add retry logic for transient failures
for (let attempt = 1; attempt <= 3; attempt++) {
try {
return await translateByGemini({...});
} catch (error) {
if (attempt === 3) throw error;
await new Promise(resolve => setTimeout(resolve, attempt * 1000));
}
}
});2. Improve Type Safety// Define proper output types
interface TranslationJobOutput {
translatedCount: number;
errorCount: number;
totalCount: number;
}
// Use in UI
{translationJob.output && (
<div className="text-sm text-muted-foreground">
Completed: {translationJob.output.translatedCount} translated,
{translationJob.output.errorCount} errors
</div>
)}3. Add Configuration// Make polling configurable
const durably = createDurably({
dialect,
pollingInterval: process.env.DURABLY_POLLING_INTERVAL ?
Number(process.env.DURABLY_POLLING_INTERVAL) : 2000,
// ...
});📋 Action Items
🚀 Deployment ReadinessReady for deployment with the understanding that:
Files Reviewed:
|
1 participant
Summary
Changes
@coji/durablyand@coji/durably-reactpackagesdurably.server.tswith translation job definition/api/durablyendpoint for handleruseJobanduseRunshooksvite-plugin-devtools-jsonfor Chrome DevTools integrationTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.