Repository navigation
feat: index questions wait until the language server has indexed, instead of answering empty or partial - #392
Conversation
The engines declare window.workDoneProgress and follow the server's $/progress (and basedpyright's `Found N source files` line) in prod_code_protocol::readiness. Index questions wait for it up to 30 s; an answer given while still indexing carries a prod-code/indexing note that the MCP tools print under the answer. The handshake's index_gated tells the client an empty answer is final. Fixes found on the way: a server request numbered like a pending question was taken for its answer (engines, client, CLI); the gateway no longer passes a server's own requests to the client; a copy is never named like a hidden directory.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a081afff9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if v.get("method").is_none() | ||
| && v.get("id").and_then(|id| id.as_i64()) == Some(2) | ||
| { | ||
| Ok(v) | ||
| } else { | ||
| Err(()) |
There was a problem hiding this comment.
Surface indexing notifications in direct CLI queries
When prod-code implementations is asked while the server is still indexing after the 30-second gate, the gateway sends a prod-code/indexing notification immediately before the response. This loop rejects every method-bearing message and discards that notification, then returns the potentially partial result without the warning that the MCP path displays. Retain and render the indexing notification while waiting for response ID 2.
Useful? React with 👍 / 👎.
| fn with_indexing_notes( | ||
| result: Result<McpToolCallResult>, | ||
| root: &Path, | ||
| ) -> Result<McpToolCallResult> { | ||
| let notes = crate::session::take_indexing_notes(root); |
There was a problem hiding this comment.
Drain indexing notes from every checkout queried
For code_schema_rename with repos, rename_across runs LSP renames with each secondary repository as the session root, so record_indexing_note stores any timeout warning under that repository. This function drains only the primary root, leaving secondary-repository warnings out of the response and allowing a partial cross-repository rename to be reported or applied silently. Associate notes with the request result or collect them for every queried root.
Useful? React with 👍 / 👎.
Problem
prod-code answered
workspace/symbol,referencesand the other index questions while the language server was still loading or indexing: empty, or with whatever it had indexed so far, and nothing said so (#391). v0.3.14's #381 only guessed, retrying an empty answer from an engine younger than 30 s, and a partial answer passed as the whole one. The servers do report when they are ready; the gateway's engines never asked for it (window.workDoneProgress) and ignored$/progress.A probe on a Linux build node, asking
workspace/symbolevery 250 ms on generated projects:Change
prod_code_protocol::readiness: aReadinesstracker fed with every message from a server. It follows what the server reports:ReadySignal::Log, the log line that says it is set up;initialized, in which work that has not begun yet is still expected.wait(max)returns when the server is ready, or returns what it is still doing when the time is up.needs_indexlists the index questions.Engines (gopls, generic):
window.workDoneProgressand feed the tracker;send_requestof an index question waits, up to 30 s (GenericLspConfig::index_wait), and attaches the unfinished work to the answer asprodCodeIndexing.Signals per server:
Found N source files/No source files found.line;Gateway:
prodCodeIndexinginto aprod-code/indexingnotification sent just before the answer;index_gated: true for the in-process Rust engine, gopls, and servers whose readiness is known.Client:
workspace_symbol_searchmakes no retry for a gated engine;Fixed on the way (each surfaced by the gateway's live Go test once progress was declared):
window/workDoneProgress/createto a pending question. The gateway passed gopls's requests on, and the client tookworkspace/configuration(id 2) for the answer to itsworkspace/symbol(id 2). Only a message without a method is an answer now, in both engines, the MCP session, the CLI's own query path and the bench. The gateway no longer passes a server's own requests on for engines that answer them.tempfiledirectory,.tmpXXXX, and its copy took that name. Go tools skip directories beginning with.or_, so gopls's firstworkspace/symbolcame backnulland only the opened file was found, 50 ms later. A probe showed only the module root's own name matters.sanitize_identifiernow names such a copydot-…/under-….Reproduce and verify
A C++ checkout of 900 files that each include
<regex>,<iostream>and<map>and callfunc0(). Two identical copies, each askedprod-code refs --symbol func0three times, 5 s apart, from its first contact with a cold clangd on the same Linux build node. 0.3.14 used the production gateway and client; this branch used its debug gateway on a spare port and the dev client:In the branch gateway's log, the first
workspace/symboltook 14,250 ms (waiting for clangd's index) and thereferencesafter it 17 ms. With 2,000 one-line files, both versions answer 997 references: that is clangd's own 1,000-result limit, so that project could not tell them apart.New tests. The two server-request tests were run against the code without their fix and failed as described; the others cover behaviour that did not exist before:
readinessunit tests (7): work begun or created but not ended is busy; a wait ends when the work ends or when the time is up; a numeric token, and an end without a begin, are handled; the pyright line; unknown servers never wait;Busyon the wire.a_question_from_the_index_waits_until_the_server_has_indexedanda_server_still_indexing_when_the_wait_ends_is_asked_with_a_note(generic engine, fake server that indexes).a_server_request_with_the_id_of_a_question_is_not_its_answer(generic engine). Without the fix, the answer is the server'screaterequest.the_first_symbol_search_of_a_fresh_gopls_finds_the_function(Go engine, real gopls).an_answer_given_while_the_server_indexes_says_soanda_server_request_with_the_questions_id_is_not_its_answer(MCP). Without the client fix: "No references found."an_empty_symbol_search_is_asked_again_only_of_an_engine_still_loadinggains the gated case: one search, however young the engine.a_server_request_is_told_from_a_notificationanda_copy_is_never_named_as_a_hidden_directory(gateway).the_handshake_says_how_old_the_engine_isassertsindex_gatedon a real gateway.the_gateway_forwards_to_a_child_language_server(live, real gopls) passes with no retry at all: 3 of 3 alone, and in the full workspace run.Checks
Linux build node (x86_64), the whole workspace in one run:
Closes #391