Skip to content

fix(server): bounded navigation wait and buffer desync detection #493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
16bit-ykiko wants to merge 1 commit into from
Closed

fix(server): bounded navigation wait and buffer desync detection #493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/server/compiler/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,19 @@ kota::task<bool> Compiler::ensure_compiled(std::shared_ptr<Session> session) {
co_return !session->ast_dirty;
}

kota::task<bool> Compiler::ensure_compiled_bounded(std::shared_ptr<Session> session,
std::chrono::milliseconds timeout) {
if(!session->ast_dirty) {
co_return true;
}
// The losing branch is dropped by when_any: dropping the sleep is
// trivially safe, and dropping the ensure_compiled frame cancels only
// this wait — the compile itself runs detached in compile_tasks.
[[maybe_unused]] auto raced =
co_await kota::when_any(ensure_compiled(session), kota::sleep(timeout));
co_return !session->ast_dirty;
}

Compiler::RawResult Compiler::forward_query(worker::QueryKind kind,
std::shared_ptr<Session> session,
std::optional<protocol::Position> position,
Expand Down
14 changes: 14 additions & 0 deletions src/server/compiler/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <chrono>
#include <cstdint>
#include <functional>
#include <optional>
Expand Down Expand Up @@ -68,6 +69,19 @@ class Compiler {
/// file_index, pch_ref, ast_deps, and publishes diagnostics.
kota::task<bool> ensure_compiled(std::shared_ptr<Session> session);

/// Bounded variant of ensure_compiled for latency-sensitive consumers
/// (index-backed navigation): wait until any compile round settles or
/// the timeout elapses, whichever comes first, and return whether the
/// session is clean. Bounded staleness — the settled round is not
/// guaranteed to be the latest (an invalidation landing mid-flight
/// keeps the flag dirty; see Session::settle_compile), and a timeout
/// abandons only this wait: the detached compile keeps running and
/// settles the session for a later request. Preemption works as for
/// any compile wait — the caller's frame may itself be cancelled by
/// $/cancelRequest, leaving the compile untouched.
kota::task<bool> ensure_compiled_bounded(std::shared_ptr<Session> session,
std::chrono::milliseconds timeout);

using RawResult = kota::task<kota::codec::RawValue, kota::ipc::Error>;

/// Forward a query to the stateful worker that holds this file's AST.
Expand Down
105 changes: 100 additions & 5 deletions src/server/service/feature_router.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "server/service/feature_router.h"

#include <algorithm>
#include <chrono>
#include <format>
#include <iterator>
#include <optional>
Expand All @@ -16,6 +17,7 @@
#include "server/compiler/indexer.h"
#include "server/protocol/serialize.h"
#include "server/protocol/worker.h"
#include "support/logging.h"
#include "syntax/completion.h"
#include "syntax/include_resolver.h"

Expand All @@ -25,18 +27,46 @@ namespace clice {

using serde_raw = kota::codec::RawValue;

/// How long index-backed navigation waits for a dirty session's compile
/// before degrading to the merged shards. Interactive navigation must not
/// hang behind a slow TU, and the shards are an acceptable bounded-staleness
/// answer. An internal constant by design, not configuration: no user knob
/// can pick a better value than "one perceptible beat".
constexpr static std::chrono::milliseconds navigation_compile_wait{1000};

/// Error response for feature requests on files with no open session.
static kota::ipc::Error document_not_open() {
return kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams, "Document not open"};
}

/// Error response for feature requests on a session whose buffer provably
/// diverged from the client's (see Session::desynced). -32801 is LSP's
/// ContentModified — the spec's "result would describe content the client
/// no longer has" code, which kota's core JSON-RPC enum does not name.
static kota::ipc::Error document_out_of_sync() {
return kota::ipc::Error{-32801, "Document out of sync"};
}

/// Error response when a call/type hierarchy item cannot be resolved back to
/// an indexed symbol.
static kota::ipc::Error item_not_resolved(llvm::StringRef kind) {
return kota::ipc::Error{kota::ipc::protocol::ErrorCode::InvalidParams,
std::format("Failed to resolve {} item", kind)};
}

kota::task<bool> FeatureRouter::await_index_freshness(std::shared_ptr<Session> session) {
if(!session || !session->ast_dirty) {
co_return true;
}
if(co_await compiler.ensure_compiled_bounded(session, navigation_compile_wait)) {
co_return true;
}
LOG_DEBUG("Navigation degrades to shards for path_id={}: no fresh compile within {}ms budget",
session->path_id,
navigation_compile_wait.count());
co_return false;
}

const std::vector<feature::DocumentLink>*
FeatureRouter::find_preamble_links(const Session& session) {
if(!session.pch_ref)
Expand Down Expand Up @@ -74,6 +104,8 @@ std::vector<protocol::Location>

kota::task<std::vector<protocol::DocumentLink>, kota::ipc::Error>
FeatureRouter::document_links(std::shared_ptr<Session> session) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto result = co_await compiler.forward_document_links(session);
if(!result.has_value())
co_return kota::outcome_error(std::move(result.error()));
Expand Down Expand Up @@ -101,19 +133,27 @@ kota::task<kota::codec::RawValue, kota::ipc::Error>
FeatureRouter::definition(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::Position& pos) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());

// Give a dirty session's compile a bounded chance to land so the
// eager answers below see the fresh file index and PCH links.
bool fresh = co_await await_index_freshness(session);

// An invalid didChange may have desynced the buffer while we waited;
// the worker forward below would serve provably diverged text.
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());

// Preamble include lines first: they have no symbol occurrence in
// the index and are invisible to the worker's AST. Dirty sessions
// skip this — the cached links may describe the pre-edit preamble —
// and retry below once the worker compile refreshed the PCH.
// skip this — the cached links may describe the pre-edit preamble.
if(session && !session->ast_dirty) {
if(auto directive = resolve_directive_definition(*session, pos); !directive.empty()) {
co_return to_raw(directive);
}
}

// Dirty sessions also skip the eager index query: resolve_cursor
// would fall back to the stale merged shard and could return a
// non-empty hit for pre-edit content, bypassing the compile below.
if(!session || !session->ast_dirty) {
auto result =
index_query.query_relations(path, pos, RelationKind::Definition, session.get());
Expand All @@ -124,6 +164,15 @@ kota::task<kota::codec::RawValue, kota::ipc::Error>

if(!session)
co_return kota::outcome_error(document_not_open());

// The bounded wait expired: degrade to the (possibly stale) merged
// shard rather than queueing behind the compile a second time — the
// forward below would wait unbounded on the same round.
if(!fresh) {
co_return to_raw(
index_query.query_relations(path, pos, RelationKind::Definition, session.get()));
Comment on lines +171 to +173

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid querying old shards with edited offsets

When this timeout branch runs after an edit inserted or deleted text before the cursor, query_relations receives the dirty session and resolve_cursor converts the current LSP position through session->line_map() before looking in the pre-edit merged shard. That mixes offsets from the edited buffer with occurrences from the old file contents, so a timed-out definition request can jump to an unrelated symbol instead of merely returning a bounded-stale/empty shard result; use the shard's own map (or avoid the shard fallback for shifted dirty buffers) on this path.

Useful? React with 👍 / 👎.

}

auto raw = co_await compiler.forward_query(worker::QueryKind::GoToDefinition, session, pos);
if(raw.has_value() && raw.value().data != "[]" && raw.value().data != "null") {
co_return std::move(raw.value());
Expand All @@ -148,32 +197,46 @@ kota::task<kota::codec::RawValue, kota::ipc::Error>

FeatureRouter::RawResult FeatureRouter::hover(std::shared_ptr<Session> session,
const protocol::Position& position) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_return co_await compiler.forward_query(worker::QueryKind::Hover, session, position);
Comment on lines +200 to 202

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-check desync after worker-backed awaits

When an invalid didChange arrives after this preflight check but while the forwarded worker request is awaiting ensure_compiled or pool.send, the guard no longer applies. forward_query can return null on the generation mismatch before the router sees session->desynced, or return the old worker result if the edit lands after the pre-send generation check, so hover/semantic tokens/etc. don't consistently report the intended ContentModified error. Re-check session->desynced after the await (or have the compiler forward helpers surface it) for these worker-backed paths.

Useful? React with 👍 / 👎.

}

FeatureRouter::RawResult FeatureRouter::semantic_tokens(std::shared_ptr<Session> session) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_return co_await compiler.forward_query(worker::QueryKind::SemanticTokens, session);
}

FeatureRouter::RawResult FeatureRouter::inlay_hints(std::shared_ptr<Session> session,
const protocol::Range& range) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_return co_await compiler.forward_query(worker::QueryKind::InlayHints, session, {}, range);
}

FeatureRouter::RawResult FeatureRouter::folding_range(std::shared_ptr<Session> session) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_return co_await compiler.forward_query(worker::QueryKind::FoldingRange, session);
}

FeatureRouter::RawResult FeatureRouter::document_symbol(std::shared_ptr<Session> session) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_return co_await compiler.forward_query(worker::QueryKind::DocumentSymbol, session);
}

FeatureRouter::RawResult FeatureRouter::code_action(std::shared_ptr<Session> session) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_return co_await compiler.forward_query(worker::QueryKind::CodeAction, session);
}

FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr<Session> session,
const protocol::Position& position) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto pause = indexer.scoped_pause();

auto path_id = session->path_id;
Expand Down Expand Up @@ -235,17 +298,23 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr<Session> sess

FeatureRouter::RawResult FeatureRouter::signature_help(std::shared_ptr<Session> session,
const protocol::Position& position) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto pause = indexer.scoped_pause();
co_return co_await compiler.forward_build(worker::BuildKind::SignatureHelp, position, session);
}

FeatureRouter::RawResult FeatureRouter::formatting(std::shared_ptr<Session> session) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto pause = indexer.scoped_pause();
co_return co_await compiler.forward_format(session);
}

FeatureRouter::RawResult FeatureRouter::range_formatting(std::shared_ptr<Session> session,
const protocol::Range& range) {
if(session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto pause = indexer.scoped_pause();
co_return co_await compiler.forward_format(session, range);
}
Expand All @@ -254,6 +323,9 @@ FeatureRouter::RawResult FeatureRouter::references(std::shared_ptr<Session> sess
llvm::StringRef path,
const protocol::Position& position,
bool include_declaration) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_await await_index_freshness(session);

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add desync re-check after bounded navigation waits

When a desyncing didChange arrives while references/declaration/type-definition/etc. are suspended in await_index_freshness, the initial guard has already run. These handlers then call IndexQuery with the now-desynced session; resolve_cursor skips the session index but still maps the cursor through session->line_map() before falling back to shards, so the request returns [] or stale locations instead of the ContentModified error. definition has the needed post-wait guard; the other index-backed handlers need the same re-check after this await.

Useful? React with 👍 / 👎.

auto locations =
index_query.query_relations(path, position, RelationKind::Reference, session.get());

Expand All @@ -275,6 +347,9 @@ FeatureRouter::RawResult FeatureRouter::references(std::shared_ptr<Session> sess
FeatureRouter::RawResult FeatureRouter::declaration(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::Position& position) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_await await_index_freshness(session);
auto locations =
index_query.query_relations(path, position, RelationKind::Declaration, session.get());
auto defs =
Expand All @@ -288,6 +363,9 @@ FeatureRouter::RawResult FeatureRouter::declaration(std::shared_ptr<Session> ses
FeatureRouter::RawResult FeatureRouter::type_definition(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::Position& position) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_await await_index_freshness(session);
co_return to_raw(index_query.query_symbol_targets(path,
position,
RelationKind::TypeDefinition,
Expand All @@ -297,6 +375,9 @@ FeatureRouter::RawResult FeatureRouter::type_definition(std::shared_ptr<Session>
FeatureRouter::RawResult FeatureRouter::implementation(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::Position& position) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_await await_index_freshness(session);
co_return to_raw(index_query.query_symbol_targets(path,
position,
RelationKind::Implementation,
Expand All @@ -307,6 +388,9 @@ FeatureRouter::RawResult FeatureRouter::call_hierarchy_prepare(std::shared_ptr<S
const std::string& uri,
llvm::StringRef path,
const protocol::Position& position) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_await await_index_freshness(session);
auto info = index_query.lookup_symbol(uri, path, position, session.get());
if(!info)
co_return serde_raw{"null"};
Expand All @@ -322,6 +406,8 @@ FeatureRouter::RawResult
FeatureRouter::call_hierarchy_incoming(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::CallHierarchyItem& item) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto info =
index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get());
if(!info)
Expand All @@ -334,6 +420,8 @@ FeatureRouter::RawResult
FeatureRouter::call_hierarchy_outgoing(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::CallHierarchyItem& item) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto info =
index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get());
if(!info)
Expand All @@ -346,6 +434,9 @@ FeatureRouter::RawResult FeatureRouter::type_hierarchy_prepare(std::shared_ptr<S
const std::string& uri,
llvm::StringRef path,
const protocol::Position& position) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
co_await await_index_freshness(session);
auto info = index_query.lookup_symbol(uri, path, position, session.get());
if(!info)
co_return serde_raw{"null"};
Expand All @@ -362,6 +453,8 @@ FeatureRouter::RawResult
FeatureRouter::type_hierarchy_supertypes(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::TypeHierarchyItem& item) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto info =
index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get());
if(!info)
Expand All @@ -374,6 +467,8 @@ FeatureRouter::RawResult
FeatureRouter::type_hierarchy_subtypes(std::shared_ptr<Session> session,
llvm::StringRef path,
const protocol::TypeHierarchyItem& item) {
if(session && session->desynced)
co_return kota::outcome_error(document_out_of_sync());
auto info =
index_query.resolve_hierarchy_item(item.uri, path, item.range, item.data, session.get());
if(!info)
Expand Down
8 changes: 8 additions & 0 deletions src/server/service/feature_router.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ class FeatureRouter {
RawResult workspace_symbol(llvm::StringRef query);

private:
/// Give a dirty session's compile a bounded chance to land before an
/// index-backed navigation query, so the query sees the fresh file
/// index instead of silently answering from stale merged shards.
/// Returns whether the session is clean afterwards; on timeout (logged)
/// the caller proceeds against the shards — bounded staleness is the
/// deliberate trade: navigation must not hang behind a slow TU.
kota::task<bool> await_index_freshness(std::shared_ptr<Session> session);

/// The preamble include links of a session's active PCH, or nullptr.
const std::vector<feature::DocumentLink>* find_preamble_links(const Session& session);

Expand Down
20 changes: 14 additions & 6 deletions src/server/service/query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ namespace lsp = kota::ipc::lsp;

void IndexQuery::visit_sessions(SessionVisitor visitor) const {
sessions.for_each([&](std::uint32_t path_id, const Session& session) -> bool {
// FIXME: when ast_dirty, consider awaiting recompilation
// instead of silently falling back to MergedIndex.
if(session.file_index && session.symbols && !session.ast_dirty) {
// Dirty sessions are filtered out and their files served from the
// merged shards: position-resolving feature entries give the
// compile a bounded chance to land first (FeatureRouter::
// await_index_freshness), so a session still dirty here degrades
// to the shards by design (bounded staleness). Desynced buffers
// are excluded outright — their index describes text the user is
// not looking at.
if(session.file_index && session.symbols && !session.ast_dirty && !session.desynced) {
return visitor(path_id, session);
}
return true;
Expand Down Expand Up @@ -78,9 +83,12 @@ bool IndexQuery::find_symbol_info(index::SymbolHash hash,
IndexQuery::CursorHit IndexQuery::resolve_cursor(llvm::StringRef path,
const protocol::Position& position,
Session* session) {
// FIXME: when ast_dirty, we fall back to MergedIndex which may be staler.
// Consider awaiting the pending recompilation to serve fresher results.
if(session && session->file_index && !session->ast_dirty) {
// The session index is consulted only while fresh; otherwise the merged
// shard answers. Position-resolving feature entries give a dirty
// session's compile a bounded chance to land first (FeatureRouter::
// await_index_freshness), so reaching the fallback dirty is the
// deliberate bounded-staleness degradation, not an oversight.
if(session && session->file_index && !session->ast_dirty && !session->desynced) {
auto map = session->line_map();
auto offset = map.to_offset(position);
if(!offset)
Expand Down
Loading