feat: meta model based proto parsing (Phase 1) - #145
Merged
Conversation
Contributor
Author
|
UPD: I've noticed that the |
coder3101
approved these changes
Jul 20, 2026
coder3101
reviewed
Jul 20, 2026
coder3101
enabled auto-merge (squash)
July 20, 2026 11:07
coder3101
changed the title
asharkhan3101
pushed a commit
that referenced
this pull request
Jul 22, 2026
Fix the metamodel query irrecoverable error handling as we discussed in #145 (comment) ### Expected behavior When executing the rewritten initialization block: ```rust let metamodel_query = Query::new(&language, &generate_metamodel_query()) .inspect_err(trace_error) .expect("Tree-sitter query compilation failed"); ``` In the highly unlikely event that `Query::new` returns an `Err` (e.g., due to a broken embedded SCM query structure), the system undergoes a graceful, multi-stage teardown instead of an abrupt termination: 1. **Synchronous Logging Dispatch (`.inspect_err`)**: The `inspect_err` combinator immediately intercepts the `QueryError` and triggers the `trace_error` closure. This formats and dispatches the details straight into the logging pipeline via `tracing::error!`. * **LSP Channel Interaction:** The `ClientLogger` captures this event and performs a `try_send` into the `tokio::sync::mpsc` buffer queue. 2. **Panicking with Context (`.expect`)**: Since the result evaluates to `Err`, `.expect()` forces the current thread to panic, carrying the message `"Tree-sitter query compilation failed"`. 3. **Runtime Interception (`CatchUnwindLayer`)**: Because the `async_lsp` server topology utilizes `CatchUnwindLayer::default()`, the panic does *not* instantly kill the OS process. * The layer catches the winding stack, safely translates the runtime failure into an appropriate JSON-RPC response or cleanly terminates the active transport streams, and allows the asynchronous loop to gracefully unwind out of `main()`. 4. **Guaranteed Buffer Flushing via RAII (`WorkerGuard`)**: As `main()` completes its termination lifecycle, Rust drops all remaining active resources in local scopes. * Crucially, the `_log_guard` variable (returned from `log::install`) is dropped. Its underlying `tracing_appender::non_blocking::WorkerGuard` destructor explicitly forces a **blocking, synchronous flush** of all remaining logs sitting in the background thread directly into `protols.log`. * This completely bypasses the need for arbitrary `std::thread::sleep(50)` pauses, guaranteeing zero diagnostic data loss. I've tested this error on VS Code client: <img width="1319" height="125" alt="image" src="https://github.com/user-attachments/assets/ceb6af80-b6cf-4372-8ca6-061f740bee46" /> and neovim: <img width="1185" height="228" alt="image" src="https://github.com/user-attachments/assets/0f92e51a-afb8-440a-982d-261032da7d35" /> So it depends on the client, whether the stderr text will be displayed. However, the log file `protols.log` includes this: > 2026-07-22T17:31:40.301120Z ERROR protols::state: Critical SCM error: Failed to compile embedded Tree-sitter query for metadata extraction. Details: QueryError { row: 0, column: 0, offset: 0, message: "invalid\n^", kind: Syntax } I expect that only `protols` developers could theoretically encounter this issue during internal development. It will never be released to regular users, as our test suite would fail the build beforehand. So it seem to be a good UX/DX.
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
2 participants
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.
Description
This PR implements Phase 1 of the high-performance meta-model refactor outlined in #130. It migrates the Language Server Protocol (LSP) hover and document symbol engines to use zero-FFI, vector-backed in-memory registries (
ParsedTree::elements) and dynamic spatial indexing.Target Branch & Release Strategy
As previously agreed, this PR targets the
nextbranch. However, if circumstances allow, I would highly recommend merging these optimizations directly intomain. Fast-tracking this cutover will significantly simplify the development of subsequent architecture phases, while allowing end-users to immediately benefit from multiple UX and performance enhancements.Key Changes
Performance Refactor: Completely eliminated FFI AST traversals during some live LSP requests. Both hover text generation and document symbol trees are now evaluated entirely in-place over pre-computed metadata within
ProtoLanguageState.Adjacency & Layout Improvements: Introduced topological sorting inside the model extractor loop to guarantee docstring retention across non-linear Tree-sitter token streams. Docstrings now comply with AIP-192 (without cross-reference support, however).
Improved hovers for user-defined fields, Well-Known and Built-In Types. Fixed bugs when some clients like VS Code displayed unexpected h2 style:
Clippy Cleanups: Fixed numerous warnings surfaced by running
cargo clippy --all-targets --all-features -- -W clippy::pedantic -W clippy::nurserywithin the touched code segments.Known Regressions & Trade-offs
Path::exists) for imported file headers has been removed from the hover tooltip. It now displays only the standard schema import path.p.exists()logic within the synchronous request thread introduces substantial I/O blocking. In repositories with--include-paths, moving the mouse over import headers can stall the main thread due to cascading file system metadata syscalls. I suggest trying to restore the path visibility during Phase 3.Test Suite Strategy
insta) have been established forproto2,proto3, andeditionssyntax variants, alongside algorithmic unit tests.src/workspace/hover.rsintentionally duplicate coverage found insrc/parser/hover.rs. These were purposefully retained to explicitly demonstrate the absence of functional regressions across the snapshot diffs (src/workspace/snapshots/) during review.Refactoring Notes
src/utils.rsfor brevity.tree_sitter::Pointandlsp_types::Positionparameters. Because these structures are small and copyable, forcing references introduces unnecessary pointer indirection and degrades compiler register allocation.Apologies for the significant volume of changes included in this single iteration.