main #1
Open
main #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
robelest
commented
Nov 3, 2025
robelest
commented
- feat: Implement semantic-release with main → prod workflow
- fix: Resolve biome linter issues
robelest
force-pushed
the
main
branch
6 times, most recently
from
9daa2f6 to
6880edf
Compare
**New Files:** - src/schemas/CRDTDelta.ts - Schema validation for CRDT deltas and stream responses - src/client/streaming/DeltaProcessor.ts - Stream-based delta processor with backpressure and rate limiting - src/client/gap-detection.ts - Gap detection and snapshot recovery utilities **Features Implemented:** - CRDT schema validation using @effect/schema - Paginated streaming with Stream.paginateEffect - Adaptive rate limiting (20/50/100 deltas/sec based on device capability) - Backpressure strategies (dropping, sliding, suspending) - Gap detection for stale checkpoints (> 7 days) - Snapshot recovery with Yjs state replacement - Fault-tolerant error recovery with automatic retries - Device capability detection (desktop/mobile/low-end) **Documentation:** - Updated EFFECT.md with Phase 3 completion summary - Removed implementation details (similar to Phase 1 & 2) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
**New Files:** - src/schemas/Document.ts - Component document schema with Yjs header validation **Modified Files:** - src/client/protocol.ts - Added Effect-based protocol validation with schemas - src/client/init.ts - Added Effect → Promise boundary functions - src/client/services/ProtocolService.ts - Exported ProtocolMismatchError **Features Implemented:** - Document schema validation with Yjs update header checks - Protocol version schema with range validation (1-99) - Effect → Promise boundaries (initializeProtocol, checkProtocolCompatibility) - Layer dependency resolution with Layer.provide - Proper error handling and type annotations **Key Fixes:** - Fixed layer dependencies (ProtocolService + IDBService) - Added explicit return types to avoid Effect type export issues - Fixed ParseError.issue (not issues) in Document validation **Architecture:** - Convex validators for API boundaries (mutations, queries) - Effect.Schema for internal validation (CRDT bytes, protocols) - No duplicate validation between layers **Documentation:** - Updated EFFECT.md with Phase 4 completion summary - Removed implementation details (similar to Phase 1-3) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
**New Files:** - src/server/errors.ts - Tagged error types (ComponentWriteError, MainTableWriteError, etc.) - src/server/mutations/insert.ts - Effect-based dual-storage insert - src/server/mutations/update.ts - Effect-based dual-storage update with optimistic concurrency - src/server/mutations/delete.ts - Effect-based dual-storage delete (hard delete pattern) - src/server/mutations/index.ts - Barrel export for mutation effects **Modified Files:** - src/server/index.ts - Exported all new error types and mutation effects **Features Implemented:** - Tagged error types using Data.TaggedError for type-safe error handling - Dual-storage insert with Yjs CRDT encoding - Dual-storage update with version-based optimistic concurrency control - Dual-storage delete with hard delete + history preservation in component - Query-first pattern for getting internal _id before patch/delete operations - Effect.withSpan for distributed tracing - Effect.logInfo for structured logging - NO retry logic inside mutations (keeps deterministic) **Key Architecture:** - Convex transaction guarantees atomicity (both writes succeed or both fail) - Component = event log (append-only, preserves history) - Main table = materialized view (efficient queries) - Yjs CRDT encoding for conflict-free updates - Type-safe with full TypeScript inference **Documentation:** - Updated EFFECT.md with Phase 5 completion summary - Removed implementation details (similar to Phase 1-4) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add one-step API builder function that creates all queries and mutations for a collection in a single call, hiding Effect.ts implementation details from users. Update EFFECT.md to mark Phase 6 as completed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove legacy code patterns and consolidate to single streamlined API: Code changes: - Remove legacy SSR array format, keep only enhanced object format - Remove Replicate class from public API (now internal to builder) - Remove dual initialization systems (initializeProtocol/checkProtocolCompatibility) - Update examples to use defineReplicate builder pattern Documentation changes: - Update README.md with simplified API (-27% lines, clearer examples) - Add comprehensive Phase 8 section to EFFECT.md documenting all cleanup - Update CLAUDE.md with modern defineReplicate patterns - Consolidate all examples to use consistent builder pattern Impact: - 87.5% reduction in setup boilerplate (8 exports → 1 function call) - Single recommended API pattern (defineReplicate only) - Cleaner public API surface (removed 8+ legacy exports) - Net -143 lines across codebase while improving clarity 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove temporary EFFECT.md documentation file that was used during Effect.ts migration planning but is no longer needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixes issue where SSR data loads instantly but disappears when hitting the Yjs/Merge layer. Two critical synchronization steps were accidentally removed during Phase 8 cleanup (commit 6520c0b): - STEP 2: Merge SSR documents into Yjs using CRDT semantics - STEP 3: Sync Yjs state to TanStack DB Root cause: SSR documents were never merged into the Yjs CRDT layer, so when TanStack DB re-rendered based on Yjs state, it showed an empty collection. The fix restores the proper data flow: 1. Wait for IndexedDB persistence 2. Merge SSR documents into Yjs (ydoc.transact) 3. Sync Yjs state to TanStack DB (begin/write/commit) 4. Reconcile with main table 5. Start subscription 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…rovements
This commit represents a major refactoring pass to improve API ergonomics,
naming consistency, and codebase clarity while removing dead code.
**Naming Improvements:**
- Renamed all "disparity/gap" terminology to "difference" for clarity
- Changed "check*" hooks to "eval*" (evalRead, evalWrite, evalRemove) for consistency
- Renamed "delete" operations to "remove" throughout (avoids reserved keyword)
- Simplified "init/initialize" patterns to "set" (setReplicate, ensureSet, SetOptions)
- Removed redundant "With" prefixes (reconcileWithMainTable → reconcile, tableWithMetadata → table)
- Renamed initialCheckpoint → origin, initialItemsForTanStack → material
- Simplified retention config: retentionDays → retention
**API Simplification:**
- Simplified convexCollectionOptions API to accept entire module:
Before: api: { stream: api.tasks.stream, insert: api.tasks.insert, ... }
After: api: api.tasks
- Added index signature to API type for cleaner interface
**Dead Code Removal (~2,830 lines):**
- Removed src/client/gap-detection.ts (175 lines, never imported)
- Removed src/client/network.ts (25 lines, unused networkStatusStream)
- Removed src/server/mutations/delete.ts (106 lines, superseded by storage.ts)
- Removed src/client/services/TabLeaderService.ts (417 lines, unused leader election)
- Removed all test files (792 lines, legacy test infrastructure)
- Removed src/schemas/CRDTDelta.ts (replaced with Delta.ts)
**File Renames:**
- src/client/init.ts → src/client/set.ts
- src/schemas/CRDTDelta.ts → src/schemas/Delta.ts
**Hook & Config Updates:**
- Updated all hooks: onDelete → onRemove, checkDelete → evalRemove
- Simplified config structure: compactionRetentionDays → compaction: { retention }
- Updated all examples to use new simplified API patterns
**Documentation:**
- Updated CLAUDE.md and README.md to reflect all naming changes
- Updated all code examples with new patterns
Total impact: 45 files changed, 470 insertions(+), 3,094 deletions(-)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…llback
The subscription callback in convexCollectionOptions was using an async
function, which broke TanStack DB's observer notification mechanism.
While begin/write/commit always triggers observers, the async keyword
caused timing/context issues that prevented useLiveQuery from
re-evaluating and React components from re-rendering on Client B.
Changes:
- Remove 'async' keyword from Convex onUpdate subscription callback
- Consolidate Effect/Schema imports to use unified 'effect' package
(import { Effect, Schema } from 'effect' instead of separate imports)
- Maintain synchronous execution flow in begin/write/commit pattern
This ensures that when Client B receives updates from Convex:
1. Delta is applied to Yjs CRDT
2. begin/write/commit executes synchronously
3. TanStack DB observers are notified correctly
4. useLiveQuery re-evaluates
5. React components re-render with new data
Tested with two browser tabs - real-time synchronization now works
correctly in both directions.
- Replace localStorage-based checkpoint functions with CheckpointService - Add loadCheckpointWithStaleDetection method to handle refresh edge cases - Fix sync bug where clients get out of sync after refresh/reconnection - Implement proper error handling with retry/timeout via IDBService - Align checkpoint architecture with existing Effect.ts patterns The stale checkpoint bug occurred when: 1. Client loads with Yjs persisting to IndexedDB 2. On refresh, Yjs loads old state from IndexedDB 3. Checkpoint from localStorage is ahead of server 4. Client misses updates between checkpoint and current state This fix detects when Yjs has persisted state but no SSR data, clears the stale checkpoint, and requests full sync from server.
…fter refresh - Remove incorrect hasPersistedYjsState parameter from loadCheckpointWithStaleDetection - Simplify checkpoint logic to trust stored checkpoints alongside Yjs persistence - Fix initial sync to use 'insert' instead of 'update' for TanStack DB population - Add try/catch fallback in initial sync for edge cases Root cause: Previous logic incorrectly cleared checkpoints when Yjs had persisted data from IndexedDB, causing full re-syncs that applied duplicate updates. Yjs CRDT merge handles duplicates as no-ops without emitting events, which prevented TanStack DB from receiving updates, breaking B->A sync after refresh. The fix recognizes that Yjs IndexedDB persistence and checkpoint tracking are independent systems that should work together, not conflict. Checkpoints track subscription position (what we've received from server) while Yjs tracks document state (CRDT). Verified against official docs: - Yjs: Updates can be applied multiple times, CRDT merge is idempotent - TanStack DB: 'insert' is correct for initial population with rowUpdateMode:'full' - Both systems now work in harmony without unnecessary checkpoint clearing
…tions - Add applyYjsInsert, applyYjsUpdate, applyYjsDelete helpers - Refactor onInsert/onUpdate/onDelete to use helpers - Encapsulate Yjs logic for better testability - All handlers now use Effect.sync pattern
- Import all services from services/index.js - Create unified servicesLayer with Layer.mergeAll - Prepare for service usage in subsequent phases
…ervice - Fix servicesLayer dependency resolution (provide IDBServiceLive explicitly) - Initialize OptimisticService with syncParams in sync function - Refactor handleSubscriptionUpdate to use Effect.gen - Use optimistic.upsert/delete/replaceAll for TanStack DB writes - Use CheckpointService for checkpoint saves - Unified error handling via Effect - Reduced manual TanStack DB write logic by ~50 lines
…nService - THE KEY FIX! - Replace manual subscription creation with SubscriptionService.create() - Replace manual reconnection with ConnectionService.startMonitoring() - Use SubscriptionService.recreate() for reconnection (ONE LINE!) - Subscription handler now returns Effect (proper SubscriptionHandler type) - Handler provides servicesLayer for service access - Remove manual event listener management - This fixes the B→A sync after reconnection bug! The key innovation: SubscriptionService stores the handler and checkpoint, so recreate() just needs the new checkpoint. The service handles cleanup and recreation automatically.
- Remove unused YjsService import (tag not needed, only Live layer) - Remove currentCheckpoint variable (written but never read) - CheckpointService manages checkpoints in IndexedDB - All lint warnings resolved - Build passes cleanly
- Convert reconcile() from async to Effect.gen - Use OptimisticService.delete() instead of manual syncParams - Wrap convexClient.query in Effect.tryPromise - Use Effect.logInfo/logError for logging - Call site updated: Effect.runPromise(reconcile().pipe(Effect.provide(servicesLayer))) - Removed 12 lines of manual TanStack DB write code
- Wrap Yjs delete and TanStack DB write in Effect.gen - Use OptimisticService.delete() instead of manual syncParams - Removed 15 lines of manual begin/write/commit code - Keep convexClient.mutation() (correct - handler's job to send deltas)
- Replace manual syncParams begin/write/commit with OptimisticService.insert() - Collect items into array then insert in one operation - Add Effect.logInfo for initial sync completion - Removed 10 lines of manual TanStack DB write code Phase 1 COMPLETE: All manual syncParams usage eliminated! OptimisticService now handles 100% of TanStack DB writes.
- Import YjsService tag for service access - Declare ydoc/ymap/persistence as let (initialized in sync function) - Use YjsService.createDocument() to create Yjs document with clientId from IDB - Use YjsService.getMap() to get Y.Map - Use YjsService.observeUpdates() for pendingUpdate observer - Setup IndexeddbPersistence after YjsService creates document - Add updateObserverCleanup to cleanup function - Eliminated all direct 'new Y.Doc()' and localStorage clientId management YjsService now manages document lifecycle and clientId persistence!
Complete service architecture integration with proper Effect-based dependency injection, deletion of orphaned code, and addition of comprehensive test coverage. **Service Integration:** - Integrate ReconciliationService for phantom document removal - Integrate SnapshotService for crash recovery - Consolidate ProtocolService as single source of truth (delete protocol.ts) - Update servicesLayer with all 9 services and proper dependencies - Remove unused withYDoc utility from YjsService **Code Cleanup:** - Delete orphaned DeltaProcessor.ts (199 lines, never imported) - Delete orphaned Document.ts schema (52 lines, never imported) - Remove duplicate protocol.ts, consolidate into ProtocolService - Delete RELEASING.md (outdated release process) **Service Improvements:** - ReconciliationService now accepts existing ydoc/ymap (no new document creation) - SnapshotService now accepts existing ydoc/ymap (no new document creation) - ProtocolService adds clearStorage() method and ensureProtocolVersion() - Updated resetProtocolStorage() to use ProtocolService (now requires params) **Bug Fixes:** - Fix version conflict detection in storage.ts (return skipped: true) - Fix IDB cleanup timing in test setup (use synchronous approach) - Fix race condition test timing tolerance (95ms instead of 100ms) - Remove non-null assertion in YjsService test **Test Suite:** - Add comprehensive test coverage for all services (133 tests) - Add test setup with fake-indexeddb - Add test utilities (effect-test-helpers, yjs-helpers, waitFor, fixtures) - Add unit tests for all 8 services - Add integration tests (conflict resolution, race conditions, two-client sync) - Configure vitest with jsdom environment and fake-indexeddb **Documentation:** - Simplify CLAUDE.md (reduce from 736 to ~200 lines) - Update with Effect-based architecture patterns - Add test command documentation - Remove outdated backwards compatibility notes **Files Changed:** - Deleted: 3 files (450+ lines) - Modified: 11 files - Added: 15 test files (3200+ lines) - Net: +2579 lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Major simplification and cleanup of the codebase: **Architecture Changes:** - Removed over-engineered Effect services (YjsService, OptimisticService, IDBService, ConnectionService, SubscriptionService) - Replaced with simple plain functions in replicate.ts and merge.ts - Reduced codebase from ~3,382 lines to ~2,689 lines (-693 lines, -20%) - Simplified services to only CheckpointService, ProtocolService, ReconciliationService, and SnapshotService **File Renames:** - sync-helpers.ts → replicate.ts (better project alignment) - yjs-helpers.ts → merge.ts (clearer CRDT intent) - OptimisticService.test.ts → replicate.test.ts - yjs-helpers.test.ts → merge.test.ts **Function Renames (sync → replicate):** - SyncParams → ReplicateParams - initializeSyncParams → initializeReplicateParams - syncInsert/Update/Delete → replicateInsert/Update/Delete - syncReplaceAll → replicateReplace **Logger & URL Updates:** - Logger categories: convex-replicate → replicate - GitHub URLs: trestleinc/convex-replicate → trestleinc/replicate - Updated package.json repository, homepage, and bugs URLs **Testing:** - All 110 tests passing - Added comprehensive test coverage for new architecture - Removed tests for deleted services 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Rename service files: remove 'Service' suffix (e.g., CheckpointService.ts → checkpoint.ts) - Rename service exports: remove 'Service' suffix (e.g., CheckpointService → Checkpoint, CheckpointServiceLive → CheckpointLive) - Rename test utility files: remove '-helpers' suffix (e.g., yjs-helpers.ts → yjs.ts) - Rename test files: kebab-case and remove '-service' suffix (e.g., CheckpointService.test.ts → checkpoint.test.ts) - Rename 'sync' to 'replicate' across all test files for consistency - Simplify function names in convex utils: waitForSubscription → wait, waitForItems → waitItems - Remove unused test utility files: effect.ts, convex.ts, wait.ts (222 lines) - Remove unused functions from collection.ts: createTestDelta, createDeltaResponse, encodeDocumentState - Remove unused error class: DeltaValidationError - Make internal test helpers non-exported (TestReplicateClient, MockReplicateServer) - Create new integration test directory with replicate-flow.test.ts All 115 tests passing, typecheck passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Switch from bun to pnpm for package management - Add .prettierrc with project formatting rules - Update dependencies to latest versions - Format all source files with prettier
Technical documentation for research paper covering: - Problem statement and system overview - Anti-pattern catalog (dual state, optimistic without rollback, etc.) - Comparison with Convex OCC, TanStack DB, Yjs transaction models - Root cause analysis of broken commits - Proposed transaction coordinator solution - Testing strategy with UI checkpoints
Server-side improvements for transaction safety: 1. Add 'sequences' table for atomic counter per collection - Leverages Convex OCC for conflict detection - Guarantees unique, monotonically increasing seq numbers - Fixes race condition in previous max(seq) approach 2. Update compaction to use snapshot boundary seq - Prevents TOCTOU race conditions - Deltas arriving during compaction won't be incorrectly included - Consistent snapshot state guaranteed 3. Add _internal flag for mark and presence mutations - Prevents direct client calls to component mutations - Security pattern for internal-only operations - Idempotent seq updates (only update if strictly greater) This is checkpoint 1: Server mutations are now ACID-compliant.
Implements a transaction coordinator inspired by: - Convex OCC: Staged changes, conflict detection - TanStack DB: Automatic rollback on error - Yjs: Transaction origins for tracking Key features: - Staged changes not visible until commit - Automatic rollback on server mutation failure - isDocumentBeingDeleted() for subscription filtering - Transaction state machine (pending → committing → committed/failed) This is checkpoint 2: Client-side transaction infrastructure.
- Replace all 'any' type annotations with proper Convex types - Use MutationCtx type for helper functions (getNextSeq, incrementDeltaCount, etc.) - Let TypeScript infer types for query builder callbacks - Replace Error with ConvexError for user-facing errors - Add SyncQueue for background task management with retry/backoff
- Add TransactionCoordinator for atomic delete operations with rollback - Capture document state before delete for potential restoration - If server mutation fails, automatically restore deleted document - Add SyncQueue for background presence marking with retry/backoff - Skip processing server changes for documents in active transactions - This prevents race conditions between local delete and server echo
The mark and presence mutations now require _internal: true to prevent direct client calls. Updated the server-side wrapper (createSessionMutation) to pass this flag when calling component mutations.
This reverts commit 0c5f269.
Component mutations are only callable via ctx.runMutation() from server code - clients cannot call them directly. The _internal flag added unnecessary complexity without security benefit.
- Remove oxlint.json from replicate and sveltekit example - Add eslint.config.js with typescript, convex, and svelte plugins - Add .prettierrc for consistent formatting - Update package versions to match ledger pnpm-workspace catalog - Add lint/format scripts matching other packages
- Remove bun tooling (bun.lock, bunfig.toml) - Remove obsolete oxlint config and CI workflow - Add pnpm-workspace.yaml with catalog for version alignment - Add root eslint.config.js with shared rules - Update .prettierrc with svelte and tailwind plugins - Convert all packages to use catalog: protocol for shared deps - Move pnpm overrides to root package.json - Add .npmrc and .prettierignore for pnpm compatibility - Update examples to extend root eslint config - Format svelte components with new prettier config
…typing - Rename examples/ to apps/ (sveltekit, tanstack-start, expo) - Extract shared convex/ backend at monorepo root with auth, schema, hooks - Add strong typing to shared validators (YjsId, RelativePosition, ProseMark, etc) - DRY refactoring: enrichChangesWithExistence, handleDocumentWrite, createOwnershipHooks - Add returns validators to all queries with typed doc validators - Remove stale re-exports (getLogger, schema re-exports) - Pin better-auth to 1.4.9 matching @convex-dev/better-auth peer - Add better-auth, zod, @trestleinc/replicate to root deps for bundler resolution - Update app collection imports to use $convex/ alias - Add shared.ts helpers (resolveUserIdentity, handleMigrationError)
Move wa-sqlite hosting from personal Cloudflare account (robelest.com) to Trestle account with edge-cached R2 bucket. Add preloadLinks() and injectPreloadLinks() utilities to eliminate the loading waterfall for wa-sqlite assets.
- Fix RelativePosition validator to use nullable wrappers (null vs undefined) - Remove transaction-guard skip in stream processing (txCoordinator check) - Seed sequence counter from existing deltas to prevent duplicate seq numbers - Regenerate component typings with updated cursor types
- Track lastProcessedSeq to skip re-processing stale deltas on subscription re-fires - Resubscribe with advanced cursor after processing so reactive query only returns new deltas - Add maxConcurrent (default: 5) to sync-queue to prevent TooManyConcurrentMutations disconnects
Replace pnpm with bun workspaces, moving catalog definitions and overrides into package.json. Adds deploy and start scripts for the Cloudflare Workers workflow.
Use a single PNG favicon instead of the old Svelte SVG and the wireframe SVG/ICO variants for consistent branding.
Pre-compile the WASM module via compileStreaming on the main thread (consuming the preload cache entry) and pass the compiled Module to the worker via postMessage, eliminating a redundant fetch+compile inside the worker and removing the preload-not-used warning.
Wrap SvelteKit auth handlers with silent retry to handle Convex cold start AbortErrors, and enable CORS on the HTTP router so the auth proxy works cross-origin from Cloudflare Workers.
Add wrangler config with preview/production environments, GitHub Actions workflow for manual deployments, pre-cache wa-sqlite CDN assets in the service worker, suppress UNUSED_EXTERNAL_IMPORT vite warning, and add lib0 + wrangler as dependencies.
…to 0 - Add VersionedSchemaBase interface (without invariant migrations() method) - Make VersionedSchema extend VersionedSchemaBase - Use VersionedSchemaBase in CollectionOptions to avoid TShape invariance - Make version optional in schema.define() (defaults to 0)
Restructure documentation to reflect the current replicate API surface: schema.define(), collection.create() unified mutation, delta/presence protocol, views, persistence providers, and experimental features.
…sts, and SSR improvements - Replace Nunito Sans/Newsreader fonts with Inter/IBM Plex Mono - Implement sharp-cornered dark theme (zero border radius) - Add virtualized sidebar and interval list via @tanstack/react-virtual - Extract Navbar and BrandIcon components, remove StarIcon and table.tsx - Consolidate Convex client into singleton (lib/convex.ts) - Switch env from VITE_CONVEX_URL to PUBLIC_CONVEX_URL with SSR guards - Use createServerFn for hydration data loading - Add error boundary to PersistenceGate with retry - Use workspace:* for @trestleinc/replicate dependency
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.