Experimental: Unified LSP server in rewatch #8243
Draft
+31,213
−4,816
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
Add optional OTLP tracing export to rewatch, controlled by the OTEL_EXPORTER_OTLP_ENDPOINT environment variable. When set, rewatch exports spans via HTTP OTLP; when unset, tracing is a no-op. Instrument key build system functions (initialize_build, incremental_build, compile, parse, clean, format, packages) with tracing spans and attributes such as module counts and package names. Restructure main.rs to support telemetry lifecycle (init/flush/shutdown) and fix show_progress to use >= LevelFilter::Info so -v/-vv don't suppress progress messages. Also print 'Finished compilation' in plain_output mode during watch full rebuilds. Introduce a new Vitest-based test infrastructure in tests/rewatch_tests/ that replaces the bash integration tests. Tests spawn rewatch with an OTLP endpoint pointing to an in-process HTTP receiver, collect spans, and snapshot the resulting span tree for deterministic assertions. Update CI, Makefile, and scripts/test.js to use the new test runner.
When stdin is a pipe (not a TTY), spawn a background thread that monitors for EOF. This allows a parent process (such as the test harness) to signal a graceful shutdown by closing stdin, without relying on signals or lock file removal.
Add mtime and content-hash based deduplication to filter out phantom and duplicate file system events. Normalize event kinds from atomic writes (temp file + rename) so they are treated as content modifications rather than create/remove cycles that trigger unnecessary full rebuilds. This fixes issues on macOS (Create events from atomic writes), Linux (duplicate inotify IN_MODIFY events), and Windows (Remove+Rename sequences from atomic writes).
On Windows, bsc writes CRLF to stdout in text mode. When the original source file uses LF line endings, the formatted output would introduce unwanted CRLF conversions. Detect the original file's line ending style and normalize the formatted output to match.
Propagate parent span through rayon in build.parse so build.parse_file spans are properly nested under build.parse instead of appearing as orphaned root spans. Enrich build.compile_file span with package, suffix, module_system, and namespace attributes for better observability. Handle invalid config changes gracefully during watch mode: replace .expect() with match to report the error and continue watching, allowing the user to fix the config without restarting the watcher.
Add 7 new fixture packages to cover more configuration dimensions: - commonjs: CommonJS module output with .bs.js suffix - namespaced: namespace package with TestNS - noop-ppx: lightweight cross-platform no-op PPX for testing - with-deps: package depending on rescript-bun for clean tests - with-dev-deps: multi-source dirs with dev dependencies - with-jsx: JSX v4 with @rescript/react - with-ppx: PPX integration using noop-ppx Enhance test helpers: - Normalize CRLF line endings in process output for Windows - Support .bs.js artifacts in sandbox cleanup detection - Add createCli, readFileInSandbox, writeFileInSandbox helpers - Add OTEL config for build.parse_file and enriched compile_file spans - Exclude noop-ppx from biome linting (CommonJS required)
Add tests for core build functionality: - Build from a package subdirectory - No stale artifacts on second build - Filter flag to compile only matching modules Add build error tests: - Parse error reporting with file location - Type error reporting - Errors when a dependency module is deleted - Circular dependency detection
Add module operation tests: - File rename with and without dependents - Duplicate module name detection - Interface file compilation and error cases Add namespace package tests: - Build with namespace flag - Namespace in compiler args - File rename in namespaced package Add dev-dependency tests: - Dev source compiles with dev dependencies - Non-dev source cannot use dev dependencies - Clean removes dev source artifacts
Add build config tests: - Experimental feature flags (valid, invalid key, invalid format) - After-build hook execution (success and failure) - Warning configuration in compiler args - Warn-error CLI override - Deprecated and unknown config field warnings Add module system tests: - CommonJS package with .bs.js suffix - CommonJS in compiler args - Suffix change triggers rebuild - Duplicate package-spec suffix error Add PPX integration tests using lightweight noop-ppx: - PPX build produces output - PPX flags in parser args - PPX flags not in compiler args Add JSX tests: - JSX v4 build with @rescript/react - JSX flags in parser args - JSX preserve flag
Add tests for scoped clean, node_modules dependency cleanup, and verifying no false compiler-update message after clean+rebuild.
Add format tests: - Stdin formatting for .res and .resi - Single file and all-files formatting - Subdirectory-scoped formatting - Check mode (pass and fail cases) Add compiler-args tests: - CWD invariance (same output from root and subdirectory) - Warning flags in both parser and compiler args
Verify that a concurrent build is prevented while watch mode holds the lock file.
Add watch mode tests: - New file creation triggers compilation - Warning persistence across incremental builds - Config change triggers full rebuild - Changes outside source dirs are ignored - Missing source folder does not crash watcher - Invalid config change recovery (watcher keeps running) - File rename removes old artifacts and compiles new file - File deletion removes artifacts
Tracing spans are thread-local, so compile_file spans created inside Rayon's par_iter had no parent connection to the compile_wave span on the main thread. Pass the wave span explicitly via `parent: &wave_span` to establish the correct parent-child relationship.
When a file is saved in the LSP, only compile the saved file and its transitive dependencies instead of every module in the project. After the initial LSP build (TypecheckOnly), all modules sit at CompilationStage::TypeChecked. A TypecheckAndEmit build targets Built, so every module would enter the compile universe. In a large project this means the first save compiles the entire codebase to JS. Fix this by computing the downward dependency closure of the saved file and temporarily promoting modules outside that closure to Built. After the incremental build, promoted modules are restored to TypeChecked. Modules already at Built from a previous save are left untouched. Also change mark_file_parse_dirty to return Option<String> (the module name) so did_save can identify the entry point for the closure walk.
package get build to js.
Add single-file typecheck on unsaved edits (didChange). The unsaved buffer content is written to a temp file in the build directory and passed to bsc directly with TypecheckOnly. Diagnostics are remapped back to the original source path. Refactor didSave into two phases: - compile_dependencies (TypecheckAndEmit): compile the saved file and its transitive imports to produce JS output. - typecheck_dependents (TypecheckOnly): re-typecheck modules that transitively import the saved file to surface errors from API changes, without emitting JS. This means saving Library.res immediately shows type errors in App.res without needing to save App.res first. Other changes: - Extract find_module_for_file helper on BuildCommandState - Add get_dependent_closure (reverse dependency traversal) - Use #[instrument] consistently for OTEL spans in the lsp/ folder - Register new OTEL spans in test-context.mjs
Add an internal `-bs-read-stdin` flag to bsc that reads source from stdin instead of from the file argument. The filename argument is still required for error locations, file kind classification, and output prefix derivation. Update the LSP didChange handler to pipe unsaved buffer content directly to bsc's stdin instead of writing temporary files to disk. This eliminates unnecessary filesystem I/O on every keystroke. Key changes: - compiler: add `Js_config.read_stdin` flag and `-bs-read-stdin` CLI option - compiler: add `Res_io.read_stdin` and `Res_driver.parse_*_from_stdin` - compiler: disable `binary_annotations` when reading from stdin (avoids Digest.file call on non-existent source file) - rewatch: replace temp file write/cleanup in did_change.rs with stdin piping
Add a new `completion-rewatch` subcommand to the analysis binary that receives all needed context (pathsForModule, opens, package config) via JSON on stdin, bypassing the expensive project discovery that the existing `completion` command performs. Analysis binary changes: - Add `CommandsRewatch.ml` with JSON parsing and package construction - Add `CompletionFrontEnd.completionWithParserFromSource` to parse from a source string instead of reading from disk - Add `Completions.getCompletionsFromSource` that takes source + package - Add `Cmt.loadFullCmtWithPackage` that uses a pre-built package record instead of calling `Packages.getPackage` Rust LSP changes: - Track open buffers in `Backend.open_buffers` (updated on didChange) - Enable completion_provider capability with trigger characters - Add `lsp/completion.rs` that builds the JSON blob with all module/ package context, spawns `rescript-editor-analysis.exe completion-rewatch`, and deserializes the LSP-conformant response - If no .cmt exists yet (completion before any didChange), run a typecheck first to produce it Test infrastructure: - Add `completeFor` helper to lsp-client.mjs - Add `lsp.completion` span to OTEL summary - Add completion integration test
Introduce AnalysisContext struct that decouples subprocess execution from the ProjectMap lock lifetime. Previously, every analysis handler held the Mutex<ProjectMap> lock through the entire analysis binary spawn + wait, serializing all LSP requests. Now the lock is only held for fast HashMap lookups, .cmt generation, and JSON context building. The expensive subprocess execution runs after the lock is released. This also eliminates ~80% boilerplate duplication across 13 handler modules by centralizing module resolution, ensure_cmt, context JSON building, and binary spawning into AnalysisContext::new() and spawn(). Other improvements: - Fix document_symbol to use open_buffers instead of reading from disk, so document symbols reflect unsaved editor content - Fix rayon span nesting by passing explicit parent span to initial_build::run, preventing work-stealing from misparenting spans - Add waitForSpan to runLspTest for stable OTEL span collection - Fix runRewatchTest waitForSpan to work with non-LSP tests - Use Value::Null instead of empty string for inlay hint maxLength
On Windows, `canonicalize()` produces verbatim paths (`\\?\D:\...`) while `read_packages` strips the prefix via `StrippedVerbatimPath`. This caused `find_independent_projects` to not recognize already-covered packages, re-discovering them as independent projects — resulting in duplicate `lsp.initial_build` spans, duplicate watcher patterns, and `watcher_count=43` instead of `35`. Apply `StrippedVerbatimPath::to_stripped_verbatim_path` to the `canonicalize()` call in `find_independent_projects` to match the path format used by `read_packages`.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
When bsc reads source from stdin (used by the LSP for didOpen/didChange typechecking), error reporting would crash with "index out of bounds" because it tried to re-read the source from disk for code frame display. The disk file content doesn't match the stdin content, causing position mismatches. Add Location.stdin_source to hold the stdin content and use it as a fallback in Location.print and error_message_utils extract_text_at_loc before falling back to disk reads. Also add LSP integration tests covering: - Type error diagnostics from buffer content before initial build - didOpen now triggers a typecheck for immediate feedback - Post-build recheck of open buffers for correct diagnostics
Replace the inline didSave build with a fire-and-forget channel send.
A background consumer task collects file paths into a HashSet, resets
a 150ms debounce timer on each arrival, and flushes one batched build
per project once the timer expires.
This handles rapid saves ("Save All", format-on-save) efficiently by
coalescing N saves into a single build, and lays the groundwork for
workspace/didChangeWatchedFiles which will send events through the
same queue.
- Add tokio sync+time features to Cargo.toml
- Create build_queue.rs with BuildQueue, debounce consumer, and
batched build logic (moved from did_save.rs)
- Change projects to Arc<Mutex<ProjectMap>> for sharing with the
spawned consumer task
- Simplify didSave handler to: tracing span + queue file path
- Extract group_by_file as a free function
- Add ProjectMap::project_root_for_path for grouping files by project
- Remove did_save.rs (absorbed into build_queue.rs)
- Update test span names and snapshot expectations
- Update LSP.md architecture docs and implementation checklist
External file changes (git checkout, terminal edits, LLM agents, formatters) now trigger incremental builds through the same debounced build queue as didSave. The handler filters for Changed events on .res/.resi files and feeds them into the build queue. Created/Deleted events are out of scope (they need module graph updates). When saving an open file, VS Code sends both didSave and didChangeWatchedFiles — the queue's HashSet deduplicates within the 150ms debounce window. Move the rewatch.lsp tracing span from the sync run_lsp wrapper in main.rs to the async run_stdio in lsp.rs. This makes it the parent of all async tasks, so build_batch spans appear at the same level as did_save and did_change_watched_files in traces. The root span is passed explicitly to BuildQueue::new, and flush_build enters it inside spawn_blocking to maintain the parent chain. - Add did_change_watched_files handler to lsp.rs - Add root_span field to Backend, captured at construction - Pass root_span through to BuildQueue for span parenting - Propagate tracing context into spawn_blocking in flush_build - Add notifyWatchedFilesChanged to LSP test client - Add writeFileExternal helper to test context - Add integration test for external file change builds - Update LSP.md: mark step 3 done, update feature table
The `compiler-args` command was superseded by the LSP's ability to compute compiler arguments on the fly. Remove the CLI entry point, the `get_compiler_args` function in `build.rs`, and the associated tests and snapshots.
Replace the build queue and synchronous didChange handler with a single unified queue that accepts all file events (didChange, didOpen, didSave, didChangeWatchedFiles). Events are consolidated per-URI with promotion rules — a typecheck is promoted to a full build when a save follows an edit within the debounce window. The queue flushes sequentially: builds first, then typechecks, then a post-build recheck for files with unsaved buffer content, then a buildFinished notification. This ensures lib/lsp/ artifacts are never written concurrently. - Create queue.rs: single consumer with 100ms debounce, per-URI event consolidation, sequential flush - Extract typecheck.rs and file_args.rs from did_change.rs - Add batched multi-file typecheck with wave-based dependency ordering - Add did_change pre-queue fallback (matching did_open behavior) - Refactor deps::read_raw_deps and compile::compiler_args for reuse - Log poisoned mutex and spawn_blocking panics instead of swallowing - Prune generation map entries after publishing diagnostics - Remove did_save.rs, did_change.rs, build_queue.rs - Update LSP.md to reflect the unified architecture - Update OTEL span names and regenerate all snapshots
Wait for publishDiagnostics after openFile in the rename test so the didOpen typecheck completes before the rename request fires. Without this, the typecheck and rename spans race, producing non-deterministic snapshot ordering. Also revert the did_change pre-queue fallback — unlike did_open, the editor only sends didChange after a file is already open, and the initial build's recheck_open_buffers already covers that window.
With the unified queue, didOpen typechecks run asynchronously via the queue consumer instead of synchronously. This means LSP requests sent immediately after openFile can race with the typecheck, producing non-deterministic span ordering in snapshots. Fix by waiting for publishDiagnostics after openFile in all tests that follow it with a request (hover, definition, rename, references, etc). This ensures the typecheck completes before the request fires.
Add a new QueueEvent::FullBuild variant to the unified queue. When didChangeWatchedFiles receives Created or Deleted events for .res/.resi files, it looks up the project root and enqueues a FullBuild. The consumer flushes full builds before incremental builds, re-initializing the affected project (re-read packages, re-scan sources, rebuild) and clearing stale diagnostics for files that no longer exist. Previously, creating or deleting a ReScript file required an LSP restart. - Refactor pending state into PendingState struct (Elm-style merge) - Add reinitialize_project mirroring initial_build::run() - Add OTEL spans for enqueue_build and enqueue_full_build events - Add integration tests for file creation (with type error) and deletion - Update LSP.md to reflect new capabilities
The LSP and standard builds previously shared lib/ocaml/ as their flat
artifact exchange directory. When the dev compiler differed from the one
that built node_modules dependencies, .cmi files would conflict and
did_save would silently fail to produce JavaScript output.
Introduce lib/lsp-ocaml/ as a dedicated flat directory for LSP builds,
mirroring what lib/ocaml/ does for standard builds. Thread BuildProfile
through all artifact path resolution so each build profile operates in
complete isolation:
- packages.rs: Add get_lsp_ocaml_build_path() and profile-aware helpers
- compile.rs: LSP uses -I ../lsp-ocaml, Standard uses -I ../ocaml
- parse.rs: Copy AST/mlmap artifacts to profile-correct flat dir
- read_compile_state.rs: Scan profile-correct flat dir for state
- clean.rs: Add clean_package_for_profile() to avoid cross-profile
destruction; clean_package now also removes lib/lsp{,-ocaml}/
- compiler_info.rs: Use profile-aware artifact check and cleanup so
a first LSP run no longer nukes lib/ocaml/
- logs.rs: Replace does_ocaml_build_compiler_log_exist with
does_flat_build_dir_have_artifacts (profile-aware)
- helpers.rs: Add get_compiler_asset_in() with explicit path param,
remove now-dead get_compiler_asset()
Add concurrent build integration test verifying LSP and CLI builds
coexist without cross-contamination.
"typechecks multiple files with cross-file consistency"
- Clean lib/lsp/ and lib/lsp-ocaml/ on LSP initial build. Orphaned .cmt files from previously deleted modules could make dependents appear up-to-date, silently swallowing compile errors (e.g. deleting App.res left Main.res without diagnostics). - Remove stale .res/.resi source copies during cleanup_previous_build. TypecheckAndEmit builds copy source files into lib/lsp-ocaml/ but cleanup never removed them when the original was deleted. - Make cleanup_after_build profile-aware. It was hardcoded to lib/ocaml/ (BuildProfile::Standard) instead of using the profile's build path, so warning-based AST cleanup targeted the wrong directory for LSP builds. - Log didChangeWatchedFiles events to the IDE output panel. Zed does not fire didChangeWatchedFiles for file deletions; these logs make it easy to diagnose missing events across editors. - Document follow-up items in LSP.md: .res.jsx cleanup on deletion, didChangeWatchedFiles reliability across editors, and LSP 3.16 file operation notifications (didDeleteFiles/didCreateFiles) as a fallback for editors that lack file watcher support.
Redesign the LSP queue from action-oriented events (Typecheck, Build, FullBuild) to fact-based events that describe what happened: - BufferOpened, BufferChanged (didOpen / didChange) - FileChangedOnDisk (didSave / didChangeWatchedFiles CHANGED) - FileCreated, FileDeleted (didChangeWatchedFiles CREATED / DELETED) The merge() function derives the correct build actions from these facts, applying promotion rules (e.g. BufferChanged + FileChangedOnDisk promotes a typecheck to an incremental build with post-build recheck). PendingState is now purely action-oriented with three clear fields: - typechecks: files needing typecheck (unsaved buffer content) - compile_files: files needing incremental build (saved to disk) - build_projects: paths requiring full project rebuild Key improvements: - FileCreated/FileDeleted evict stale per-file entries from typechecks and compile_files since the full rebuild will cover them - Merged FileSaved and FileChangedOnDisk into a single variant — they carried the same payload and had identical merge behavior - Removed deleted_uris tracking from PendingState — the old/new URI diff after reinitialize_project naturally clears stale diagnostics - File creation/deletion events now carry only file_path (no URI), and project root resolution happens at flush time Split the ~1630-line queue.rs into focused submodules: queue.rs — types, public API, consumer, merge, flush orchestration queue/file_build.rs — per-file incremental build (dependency + dependent closure) queue/file_typecheck.rs — per-file typecheck (parallel, wave-based, staleness-aware) queue/project_build.rs — per-project full rebuild + artifact cleanup Added tests for file deletion cleanup (.res deletes .resi + JS, .resi deletion is a no-op) and comprehensive merge unit tests covering all promotion rules and cross-cutting scenarios (93 unit tests, 101 integration tests).
Replace rayon `into_par_iter` with `std::thread::scope` for workspace-group-level parallelism in the initial build. Each workspace build uses rayon internally for file-level parallelism (parse, compile), where tasks block on `bsc` subprocess I/O via `Command::output()`. Nesting this inside an outer `par_iter` on the same global thread pool caused thread starvation: outer tasks occupied all rayon threads, leaving none for inner tasks to make progress. Using dedicated OS threads for the outer level avoids this — they don't consume rayon pool slots, so the full pool remains available for the inner build parallelism.
When `diagnostics_http: true` is set in initializationOptions, the LSP spawns a lightweight HTTP server on a random free port. External tools (e.g. LLM agents) can query `GET /diagnostics` to retrieve current compiler diagnostics without triggering a separate build. The endpoint blocks until the LSP is idle (no pending events or flushes in progress), with a 30-second timeout. A pending-ID model with a FlushGuard ensures correct idle detection even when events arrive during a flush or a flush panics.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This branch explores embedding a full LSP server directly into the
rescriptbinary (rescript lsp), replacing the current architecture where a Node.js extension mediates between the editor and separate build/analysis processes.The core idea
Today, the ReScript editor experience involves three processes: a Node.js VS Code extension, the
rescriptbuild watcher, and therescript-editor-analysis.exebinary. They communicate through files on disk — the editor extension launches builds, waits for artifacts, then shells out to the analysis binary for each request.This branch collapses the build system and LSP server into a single Rust process using
tower-lsp. The build state lives in memory, and analysis requests shell out to the samerescript-editor-analysis.exebut with source code passed via stdin instead of being read from disk.No temp files — stdin everywhere
Both
bscand the analysis binary receive source code via stdin rather than through temporary files. FordidChange(unsaved edits),bsc -bs-read-stdinproduces diagnostics without writing anything to disk. For analysis requests (hover, completion, code actions, etc.), the analysis binary receives a JSON blob on stdin containing the source text, cursor position, and package metadata. The OCaml analysis code was refactored withFromSourcevariants that parse from a string rather than opening files — so everything works correctly on unsaved editor buffers.Separate build profile:
lib/lspThe LSP server writes its build artifacts to
lib/lsp/instead oflib/bs/. This means it doesn't conflict withrescript buildorrescript build -wrunning in a terminal — both can operate independently on the same project without stepping on each other's artifacts.Initial build: typecheck only
On
initialized, the server runs a full build but only goes as far as producing.cmt/.cmifiles (theTypecheckOnlyprofile). It deliberately skips JS emission. This gets the editor operational as fast as possible — type information for hover, completion, go-to-definition etc. is all available, without paying the cost of generating JavaScript for every module upfront.Smart incremental builds on save
When a file is saved, the server runs a two-phase incremental build:
Emit JS for the dependency closure — the server computes the transitive imports of the saved file and only emits JavaScript for that file and its dependencies. Modules outside this closure are skipped entirely. So saving a module produces JS for it and any imports that haven't been compiled yet — not the entire project.
Typecheck reverse dependencies — modules that transitively depend on the saved file are re-typechecked to surface errors caused by API changes (e.g. a removed export). This gives you project-wide diagnostics on save — if you rename a function, you immediately see errors in every file that uses it, even files you don't have open. No JS is emitted for these — they get their JS when they are themselves saved.
What's implemented
All standard analysis endpoints are wired up: completion (with resolve), hover, signature help, go to definition, type definition, references, rename (with prepare), document symbols, code lens, inlay hints, semantic tokens, code actions, and formatting.
Observability
Every LSP request and build operation is traced with OpenTelemetry spans, viewable in Jaeger. This makes it straightforward to profile request latency and understand what the server is doing.
Test infrastructure
Each endpoint has integration tests using
vscode-languageserver-protocolthat boot a real LSP server in a sandbox, send requests, and snapshot both the results and the OTEL trace structure.What's not here yet
workspace/didChangeWatchedFiles— handling external file changes (git checkout, etc.)createInterfaceandopenCompiledcustom commandsThis is an experiment to validate the architecture. If it proves useful, individual pieces can be split into focused PRs.