Refactor specs and add tooling configuration#2
Conversation
## Specification changes - Rename docs/durably.md -> docs/spec.md - Add docs/spec-streaming.md for AI Agent streaming support - Add docs/implementation-plan.md with TDD roadmap - Unify package naming to @coji/durably - Migrate from better-sqlite3 to Turso/libsql - Add jobName field to Step events - Add name/$types to JobHandle in v2 spec ## Tooling - Add biome for linting - Add prettier with organize-imports plugin - Update README.md and CLAUDE.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. WalkthroughAdds formatting and project configuration files, updates docs and README to rename APIs and switch SQLite dialects to LibSQL/Turso, and introduces detailed specification and implementation-planning documents for streaming and monorepo development. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
22-25: Missing lint and format scripts.The PR objectives state that
pnpm lintandpnpm formatpass checks, but these scripts are missing from thescriptssection. Without them, users cannot run the linting and formatting tools that were added as devDependencies.🔎 Add missing scripts
"scripts": { "build": "tsup", - "prepublishOnly": "pnpm build" + "prepublishOnly": "pnpm build", + "lint": "biome check .", + "format": "prettier --write ." },
🧹 Nitpick comments (4)
docs/spec.md (1)
161-162: Consider documenting cleanup strategy for idempotencyKey records.The spec states that
idempotencyKeyhas no expiration and remains valid as long as the Run exists. While this is correct behavior, it could lead to unbounded database growth over time if old Runs are not actively managed.Consider adding a note in the documentation about:
- Recommended cleanup strategies for old Runs
- TTL or archival policies applications should implement
- Storage growth implications for high-volume scenarios
docs/spec-streaming.md (2)
176-194: Note potential confusion with column naming.Line 193 explicitly mentions that the DB column name is
created_atbut will be exposed astimestampin the event type to match the v1 pattern. While this maintains consistency with v1, it could create confusion during debugging or when writing raw SQL queries.Consider adding a comment in the actual schema migration noting this mapping for future maintainers.
407-409: Consider addressing authorization strategy for subscribe().Line 408 raises an important security question: "Should anyone who knows the runId be able to subscribe?" but leaves it unresolved.
For the specification document, consider adding:
- A proposed authorization model (e.g., token-based, same-origin only, etc.)
- Guidance on what applications should implement
- Security implications of different approaches
This doesn't need to be implemented in v1, but having a planned approach would help guide future implementation.
.prettierignore (1)
1-1: Consider adding common ignore patterns.While ignoring
pnpm-lock.yamlis correct, consider adding other common patterns for completeness:
dist/(build output)node_modules/(dependencies, though often ignored by default)coverage/(test coverage reports).git/(version control)🔎 Suggested additions
pnpm-lock.yaml +dist/ +coverage/
📜 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 (11)
.prettierignore(1 hunks).serena/.gitignore(1 hunks).serena/project.yml(1 hunks)CLAUDE.md(2 hunks)README.md(1 hunks)biome.json(1 hunks)docs/implementation-plan.md(1 hunks)docs/spec-streaming.md(1 hunks)docs/spec.md(13 hunks)package.json(1 hunks)prettier.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Define jobs usingdefineJob()function that receives a context object and payload
Create steps viactx.run(), with each step's success state and return value persisted automatically
Create job execution instances (runs) viatrigger()API, which always persists runs aspendingbefore execution
Use explicit failure handling with no automatic retry - utilize theretry()API for manual retry logic
Use dialect injection pattern by passing Kysely dialect tocreateClient()to abstract SQLite implementations
Extend framework behavior using the event system with events:run:start,run:complete,run:fail,step:*,log:write
Implement optional features using the plugin architecture viaclient.use()method, such as log persistence
Use default configuration values:pollingInterval1000ms,heartbeatInterval5000ms,staleThreshold30000ms for detecting abandoned runs
Files:
prettier.config.js
🧠 Learnings (11)
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use default configuration values: `pollingInterval` 1000ms, `heartbeatInterval` 5000ms, `staleThreshold` 30000ms for detecting abandoned runs
Applied to files:
prettier.config.jsbiome.json
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Durably is a step-oriented batch execution framework for Node.js and browsers with resumable batch processing using only SQLite for persistence
Applied to files:
CLAUDE.mddocs/spec.mdREADME.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: The same job definition code must run in both Node.js and browser environments - Node.js uses better-sqlite3/libsql, browsers use SQLite WASM with OPFS backend
Applied to files:
CLAUDE.mddocs/spec.mdREADME.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*migration*.{ts,tsx,js,jsx,sql} : Database schema contains four tables: `runs`, `steps`, `logs`, `schema_versions`. Runs have fields: `status` (pending/running/completed/failed), `idempotency_key`, `concurrency_key`, `heartbeat_at`. Steps have fields: `status` (completed/failed), `output` (JSON), indexed by `run_id` and `index`
Applied to files:
CLAUDE.mddocs/spec.mdREADME.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Create job execution instances (runs) via `trigger()` API, which always persists runs as `pending` before execution
Applied to files:
CLAUDE.mddocs/spec.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Define jobs using `defineJob()` function that receives a context object and payload
Applied to files:
CLAUDE.mddocs/spec.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Create steps via `ctx.run()`, with each step's success state and return value persisted automatically
Applied to files:
CLAUDE.mddocs/spec.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Database persistence uses SQLite with dialect abstraction through Kysely for both Node.js (better-sqlite3/libsql) and browser (SQLite WASM) implementations
Applied to files:
CLAUDE.mddocs/spec.mdREADME.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use dialect injection pattern by passing Kysely dialect to `createClient()` to abstract SQLite implementations
Applied to files:
CLAUDE.mddocs/spec.mdREADME.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Extend framework behavior using the event system with events: `run:start`, `run:complete`, `run:fail`, `step:*`, `log:write`
Applied to files:
docs/spec.md
📚 Learning: 2025-12-09T04:34:39.625Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T04:34:39.625Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Implement optional features using the plugin architecture via `client.use()` method, such as log persistence
Applied to files:
docs/spec.md
🔇 Additional comments (17)
.serena/.gitignore (1)
1-1: LGTM!Standard ignore pattern for cache directory.
CLAUDE.md (2)
7-15: LGTM! Documentation accurately reflects the architectural updates.The updates to runtime libraries (Turso/libsql, SQLocal) and documentation structure are clearly documented and consistent with the PR objectives.
19-30: LGTM! API naming updates are consistent.The changes from standalone functions to method-based API (
durably.defineJob(),createDurably(),durably.use()) improve discoverability and namespace clarity.docs/implementation-plan.md (3)
869-879: LGTM! Proper consideration of OPFS cleanup challenges.The documentation correctly identifies that OPFS database files may require explicit deletion and notes this as a consideration for browser testing. This is an important operational concern for test cleanup.
328-364: LGTM! Storage abstraction supports v2 extensibility.The Storage interface design with optional methods (e.g.,
createLog?,getLogs?) and clear separation of Run, Step, and Log operations provides a solid foundation for future v2 features while maintaining clean boundaries.
198-206: No changes needed—LibSQL supports ':memory:' syntax for in-memory databases.LibSQL supports ':memory:' for in-memory databases, consistent with SQLite's API. The libsql C API documentation explicitly shows ':memory:' as valid syntax, and the libsql npm package includes examples using ':memory:'. The code syntax is correct.
Note: There are known issues with transactions in ':memory:' databases in the libsql-client-ts library where in-memory databases can be discarded after transaction creation, but this may not affect simple test scenarios.
.serena/project.yml (1)
16-84: LGTM! Well-documented project configuration.The configuration appropriately sets TypeScript as the primary language, enables gitignore integration, and includes helpful inline documentation for all available options.
README.md (1)
11-54: LGTM! README accurately reflects the new API surface.The migration from
better-sqlite3to LibSQL/Turso is clearly documented, and the API examples correctly show:
- New import paths (
@libsql/kysely-libsql)- Object-based job definition syntax
- Environment-based configuration pattern
docs/spec.md (4)
327-336: LGTM! Addition of jobName to step events improves consistency.Including
jobNamein step events (lines 327-336) ensures all events carry sufficient context for filtering and type-safe event handling, which aligns well with the Discriminated Union pattern described later.
341-427: LGTM! Well-structured event type definitions.The formal event type definitions with:
- Discriminated Union pattern via
typefieldsequencefield for ordering guarantees- Comprehensive interface definitions
This provides a solid foundation for type-safe event handling and future v2 streaming features.
839-967: LGTM! Comprehensive v2 preparation guidance.The internal design guidelines section provides clear, actionable guidance for:
- JobContext extensibility (class/factory pattern)
- Storage abstraction
- Event sequencing
This forward-thinking approach will minimize breaking changes when v2 features are added.
667-668: LGTM! Appropriate ULID implementation choice.Using
ulidxis a good choice as it's lightweight, supports both Node.js and browsers, and provides the needed sortability and uniqueness properties.docs/spec-streaming.md (3)
82-141: LGTM! Excellent use of Web Streams API for cross-platform streaming.Using
ReadableStreamas the return type forsubscribe()is a smart choice because:
- Native browser API (no additional dependencies)
- Supports backpressure
- Works in both Node.js and browsers
- Enables
for await...ofconsumptionThe implementation pattern with
start()for initial events andpull()for new events is well-designed.
354-398: LGTM! Well-justified trade-off for event persistence strategy.The decision to persist only coarse-grained events (step:, run:) while keeping token-level emits in-memory is pragmatic and well-reasoned:
- Dramatically reduces DB write load
- Token-level data is regenerable (LLM responses)
- Reconnection still works at step boundaries
- Documented trade-offs are acceptable
The implementation rationale and reconnection behavior are clearly explained.
416-514: LGTM! Comprehensive and realistic use case example.The AI Coding Assistant example effectively demonstrates:
- Mixing
ctx.run()andctx.stream()- Different emit types for various streaming data
- Progress tracking integration
- Client-side stream consumption patterns
This provides clear guidance for implementers and users of the streaming API.
prettier.config.js (1)
1-7: LGTM!The Prettier configuration is well-structured with sensible defaults:
- No semicolons and single quotes align with modern JavaScript conventions
- Trailing commas improve git diffs
- 80-character print width is a reasonable default
- The organize-imports plugin integration will help maintain consistent import ordering
package.json (1)
55-58: No action needed. prettier-plugin-organize-imports@4.3.0 has peer dependency prettier>=2.0, and prettier@3.7.4 satisfies this requirement.
| "files": { | ||
| "includes": ["**", "!worker-configuration.d.ts", "!app/styles/globals.css"] | ||
| }, | ||
| "assist": { "actions": { "source": { "organizeImports": "off" } } }, |
There was a problem hiding this comment.
Clarify responsibility for import organization.
Biome's organizeImports is disabled here, but according to the AI summary, prettier-plugin-organize-imports is enabled in the Prettier config. This creates ambiguity about which tool manages import organization.
Consider documenting which tool is responsible or removing one of the conflicting configurations to avoid confusion.
🤖 Prompt for AI Agents
In biome.json around line 6, the project disables Biome's organizeImports while
Prettier enables prettier-plugin-organize-imports, causing ambiguity about which
tool should manage imports; either remove/disable one of the conflicting
settings or add a clear comment and documentation indicating the chosen tool for
import organization (e.g., keep Prettier plugin enabled and re-enable/clarify
Biome setting or leave Biome off and document that Prettier handles imports) and
update config files/readme accordingly so only one tool is authoritative.
1 participant
Summary
docs/durably.md->docs/spec.mdfor claritydocs/spec-streaming.mdfor AI Agent streaming support (v2)docs/implementation-plan.mdwith TDD roadmap@coji/durablyjobNamefield to Step events for consistencyname/$typesto JobHandle in v2 specTooling
pnpm lint)pnpm format)Test plan
pnpm build)pnpm lint)pnpm format)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.