Experimental: Unified LSP server in rewatch#8243
Draft
nojaf wants to merge 119 commits intorescript-lang:masterfrom
Draft
Experimental: Unified LSP server in rewatch#8243nojaf wants to merge 119 commits intorescript-lang:masterfrom
nojaf wants to merge 119 commits intorescript-lang:masterfrom
Conversation
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
- Centralize file classification helpers (is_rescript_source, is_rescript_config, is_rescript_file) in file_args.rs - Guard did_open, did_change, did_save to skip non-ReScript files - Route rescript.json saves to full project rebuilds via new ConfigChanged queue event - Add LSP integration tests for config change handling - Update LSP.md: note ACP diagnostics discussion, update Next Up items
Wait for both publishDiagnostics and buildFinished concurrently using Promise.all, so the test completes regardless of which notification arrives first. Filter timing-dependent lsp.typecheck spans from the snapshot to handle the two possible execution orders.
ConfigChanged now clears pending typechecks for the affected project, since the full TypecheckOnly rebuild already covers them. Only files under the config's project root are cleared to avoid discarding work for unrelated projects in a monorepo. compile_files are kept because the full rebuild doesn't emit JS.
When a file is saved that starts importing a new module (e.g., one created externally by a shell command or LLM agent), the dependency closure was computed from stale module deps. This excluded the new dependency from the compile universe, causing "module not found" errors. Pre-parse dirty files and resolve their deps in compile_dependencies before computing the closure, so the saved content's imports are reflected in the compile universe.
`rescript clean` was removing lib/lsp/ and lib/lsp-ocaml/, which are owned by a running LSP server. This caused panics and broken IDE features when the LSP tried to use deleted artifacts. Also remove panicking .expect() calls when copying source files during compilation — if a file is deleted between build start and copy (common in watch/LSP), the build continues gracefully instead of crashing. Add a `cli` helper to `runLspTest` context so LSP tests can invoke rescript CLI commands without manual sandbox lifecycle management. Refactor concurrent-build tests to use it, and add a new clean test verifying the LSP survives `rescript clean`.
Replace BuildProfile with BuildConfig { OutputTarget, CompileScope },
and split incremental_build into separate parse and compile phases.
Build pipeline:
- Extract parse_and_resolve() for parsing and dep resolution
- Extract compute_compile_universe() for universe computation
- incremental_build() now takes an explicit compile universe
- All callers follow: parse → compute universe → compile
Compile loop correctness:
- Only dirty dependents on successful compilation
- Snapshot/restore module stages to prevent orphan dirty leaks
- Remove post-hoc cleanup from LSP typecheck_dependents
LSP simplification:
- Eliminate duplicate parsing in compile_dependencies
- Remove verbose debug logging from file_build.rs
- Handle parse error diagnostics in initial_build and project_build
Add dep-chain test fixture and stale-dirty regression test.
Update LSP.md build flow documentation.
Introduce a `CompileUniverse` struct that holds both the full set of modules participating in a compile cycle (`all`) and the subset that were directly dirty (`originally_dirty`). This replaces the bare `AHashSet<String>` parameter and removes the architectural mismatch where per-module `compilation_stage` mutations inside the loop re-derived the same information. Key changes: - Replace the `needs_compile` gate with `clean_modules`-based logic: a module skips compilation if it was not originally dirty and all its in-universe dependencies had unchanged `.cmi` output. - Remove in-loop dirtying of dependents (`compilation_stage = Dirty`). - Remove `pre_loop_stages` snapshot/restore (no longer needed without in-loop mutations). - Move stage/timestamp updates to a post-loop pass that runs even when the loop breaks early due to compilation errors. - Remove LSP compensating hacks: `typecheck_dependents` dirtying, and `Built → TypeChecked` downgrades in initial and project builds. - Replace the 5-element result tuple with `CompileModuleResult` struct and `CompileFileOutcome` enum for readability.
Rename `needs_deps_rescan` to `needs_dependencies_rescan` across all usage sites. Add doc comments to every field in the Module struct, organized into three sections: - Module identity (immutable after construction) - Dependency graph (mutated in deps.rs after each AST rescan) - Build status (mutated throughout the build pipeline) Each comment describes the field's purpose and where it is mutated.
CompileUniverse CompileScope now carries anchor module names in its scoped variants (CompileDependencies, TypecheckDependents) as AHashSet<String>, and incremental_build derives the CompileUniverse internally via compute_universe_for_scope. This eliminates the pattern where every caller had to construct a matching CompileUniverse separately — a mismatch would silently produce wrong results. Other changes: - Move dependency_closure.rs from lsp/ to build/ (no LSP dependency) - Change compile::compile() to borrow &CompileUniverse (zero clones) - Return modules in IncrementalBuildResult/Error for caller use - Use AHashSet<String> consistently in closure function signatures - Update LSP test snapshots for new typecheck_dependents trace spans
Consolidate show_progress, plain_output, initial_build, and
only_incremental into an OutputMode enum with two variants:
- Standard { show_progress, plain_output, initial_build } for
CLI/watcher
- Silent for LSP (no user-facing output)
Make create_sourcedirs unconditional — it now always runs and writes
to the correct output folder (lib/bs or lib/lsp) based on OutputTarget.
Derive only_incremental internally from !initial_build, removing it
as a parameter entirely.
This reduces incremental_build from 8+ parameters down to 4 and makes
LSP call sites cleaner — they just use OutputMode::Silent without
needing to think about display settings.
write_build_ninja - Take incremental_build's build_config by reference (&BuildConfig) instead of by value, consistent with parse_and_resolve - Gate logs::initialize/finalize on OutputMode (is_silent) instead of OutputTarget, decoupling artifact location from display behavior - Guard all eprintln! calls in parse_and_resolve and incremental_build with !is_silent() so LSP builds never leak to stderr - Remove redundant show_progress parameter from compile::compile; it already receives &BuildConfig and can read it from there - Skip sourcedirs::print on scoped LSP builds (CompileDependencies, TypecheckDependents) where no files were added or removed - Remove write_build_ninja entirely (no longer needed)
After the LSP's initial TypecheckOnly build, .cmi/.cmt files exist but no .cmj is produced. cleanup_previous_build incorrectly marked these modules as Built (based solely on .cmt freshness), so the first didSave would skip compiling dependencies — failing when .cmj was needed. Fix: read_compile_state now tracks .cmj files. cleanup_previous_build marks modules as TypeChecked (not Built) when .cmj is missing, ensuring the full dependency closure gets compiled on the first save. Also: - Add lsp.flush span with file/project details for OTEL traces - Rename LSP build spans to lsp.flush.* hierarchy for clarity - Add error_count tracking on file_build spans - Add graceful SIGTERM/SIGINT shutdown handling for LSP server - Improve test snapshot readability with flush span grouping - Add regression test (initial-save-cmj) for the .cmj fix
Add a local OTLP trace viewer (rewatch/otel-viewer/) that replaces the Jaeger Docker workflow. Features include a focused span tree UI, LLM export, and deep links to individual traces via the Navigation API. The rewatch integration test infrastructure can now forward traces to otel-viewer via OTEL_VIEWER_ENDPOINT env var, fire-and-forget alongside the existing in-process span collection for snapshot assertions. - Add otel-viewer: Python/FastAPI OTLP collector with SQLite + web UI - Add client-side routing with Navigation API for trace permalinks - Auto-expand first root span when opening a trace - Add .gitignore for otel-viewer artifacts - Add trace forwarding to test OTEL receiver - Update AGENTS.md to recommend otel-viewer over Jaeger - Document uv setup, test trace viewing, and why not Jaeger
Use a take-build-replace pattern to minimize the time the ProjectMap mutex is held during incremental builds. Previously the lock was held for the entire duration of bsc subprocess invocations (AST generation, compilation, typecheck of dependents). Now the BuildCommandState is removed from the map under a brief lock, built without holding the mutex, then inserted back. This unblocks LSP handlers (hover, completions, go-to-definition, etc.) during save-triggered builds. Handlers for the affected project will return empty responses instead of blocking on the mutex.
Destructure BuildState to get separate borrows of `packages` / `project_context` (read-only) and `modules` / `module_names` (mutable), removing the `build_state.packages.clone()` workaround. This avoids deep-cloning the entire AHashMap<String, Package> on every build initialization.
Use separate span names for compile and typecheck operations: - build.compile / build.compile_wave / build.compile_file for full builds - build.typecheck / build.typecheck_wave / build.typecheck_file for typecheck-only Also add scope and output attributes to the incremental_build span, and Display impls for OutputTarget and CompileMode.
hashes
CompilationStage now carries blake3 hashes at each stage:
Dirty → Parsed { source_hash, ast_hash }
→ TypeChecked { +cmi_hash, +cmt_hash }
→ Built { +cmj_hash }
This fixes the LSP performance issue where saving a single file caused
all modules to be recompiled. After the initial FullTypecheck build,
every module sat at TypeChecked. CompileDependencies with target Built
would mark all of them as originally_dirty since TypeChecked < Built.
Now CompileDependencies uses is_dirty() for originally_dirty, so only
modules whose source actually changed are marked dirty. The skip logic
checks needs_compile_for_mode() to determine if a non-dirty module
already reached the target stage. On the second save, all 444 unchanged
modules are skipped in sub-millisecond time.
Also fixes a pre-existing bug in mark_modules_with_expired_deps_dirty
where both sides of a tuple checked last_compiled_cmt instead of
last_compiled_cmi and last_compiled_cmt.
modules Replace the flat Module struct + SourceType discriminator with a proper enum: Module::SourceFile(SourceFileModule) and Module::MlMap(MlMapModule). This ensures compilation-stage fields (compilation_stage, last_compiled_cmi, last_compiled_cmt, is_type_dev, needs_dependencies_rescan) only exist on source file modules, making it a compile error to access them on mlmap modules. Removes the fake blake3::hash(b"mlmap") sentinels that were previously stuffed into CompilationStage::Built to satisfy the type system. Accessor methods on Module (package_name, deps, dependents, get_interface, needs_compile_for_mode) provide clean access to shared fields without requiring callers to match on the variant. In parse.rs, when mlmap output changes, dependents are now directly marked dirty instead of setting a compilation_stage on the mlmap module itself.
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
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.