Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/agent/implement.md`:
- Line 201: The SmallVector declaration in ServerPathPool is missing template
parameters; update the SmallVector usages (e.g., the members named path_ids or
paths inside the ServerPathPool implementation) to use the correct template form
llvm::SmallVector<ElementType, N> (for example llvm::SmallVector<std::uint32_t,
8> for stored IDs or llvm::SmallVector<llvm::StringRef, 8> for stored path
strings) so the definitions compile; ensure any forward declarations or typedefs
referencing SmallVector are similarly corrected to the templated form.
- Around line 426-458: Replace the manual doc.strand_mutex.lock()/unlock() in
StatefulWorker::on_compile with an RAII guard that acquires the mutex and
releases it on scope exit (use std::lock_guard/std::scoped_lock or the eventide
awaitable lock variant that returns a guard) so the mutex is always released
even if co_await et::queue throws; apply the same change to the on_hover example
(replace explicit lock/unlock with the same RAII guard for doc.strand_mutex) and
ensure the guard type is compatible with the async/await pattern used by
et::queue.

In `@docs/agent/note.md`:
- Around line 143-145: The note "Decision: Global queries not wired in this
phase" is confusing relative to later text that says indexing and cross-file
features are integrated; update the wording to clarify this is historical
context for a specific phase (or else reconcile the status). Edit the paragraph
referencing StatelssWorker `on_index` and IndexScheduler in the implement.md
note to explicitly state "at that phase" or "historical — before TUIndex
extraction was wired" (or flip to state the current integrated status if
indexing is done), so readers won't see a contradiction with the later section
that describes indexing/cross-file features as integrated.
- Line 161: Update the docs to correct the Linux procfs path used by
get_process_memory: replace the incorrect "/proc/statm" reference with the
per-process path "/proc/self/statm" (or "/proc/<pid>/statm") and note that
WorkerPool must expose worker PIDs before the periodic monitoring loop can read
those per-process statm files; ensure the description in get_process_memory
mentions the platform-specific use of macOS task_info and Linux /proc/self/statm
for clarity.

In `@src/server/cache_manager.cpp`:
- Around line 98-103: The loop in cache_manager.cpp decrements total_size
unconditionally after calling llvm::sys::fs::remove, which can make the cache
appear smaller even if remove failed; modify the loop to capture the remove
result (std::error_code ec = llvm::sys::fs::remove(entry.path)), only subtract
entry.size and emit LOG_INFO("Evicted...") when ec indicates success, and on
failure emit a LOG_WARN/LOG_ERROR including ec.message() and entry.path and skip
decrementing total_size so the eviction loop continues correctly until disk
usage is under max_size_; keep the break condition using total_size <= max_size_
as-is.
- Around line 24-25: The call to llvm::sys::fs::create_directories(cache_dir_)
currently ignores errors so unwritable paths are treated as ready; update the
CacheManager initialization to check the return status of
create_directories(cache_dir_), and if it fails log the error (include the error
code/message) instead of the successful LOG_INFO("Cache directory: {}"); then
disable caching (e.g. set a cache_enabled_ flag to false) or propagate the
failure/return early so subsequent PCH/PCM writes won't assume the directory
exists. Ensure you reference cache_dir_, create_directories(cache_dir_), and
replace the current LOG_INFO with a LOG_ERROR/LOG_WARN that includes the error
details.

In `@src/server/compile_graph.cpp`:
- Around line 33-61: The remove_unit implementation currently erases the unit
while coroutines (compile_deps()/compile_impl()) may be suspended waiting on the
unit's completion event, causing use-after-free; change remove_unit to first
mark the unit as canceled (e.g., set a removed/canceled flag on the Unit entry
and signal its completion/event to wake waiters) and do NOT erase the map entry
or destroy the event immediately — instead decrement/observe an in-flight
counter or attach a deferred cleanup list and only physically erase the units
map entry and remove from pch_cache/pcm_cache after all in-flight compiles
(tracked by an in_flight counter or waiter count on the Unit) have finished;
reference CompileGraph::remove_unit, Unit::completion (or completion),
compile_deps(), compile_impl(), and the pch_cache/pcm_cache removals when making
this change.
- Line 101: The cycle-detection stack is never armed because top-level calls to
compile_impl are scheduled with stack == nullptr; change the loop.schedule(...)
invocations that currently call loop.schedule(compile_impl(dep_id, loop)) so
they construct and pass a real stack object (e.g., a newly created shared_ptr or
unique_ptr to the stack/vector type used by compile_impl) instead of nullptr,
and ensure every internal call to compile_impl propagates that same stack
pointer so the guard inside compile_impl (the stack check and cycle-reporting
code) can run and break dependency cycles rather than leaving units waiting on
each other's completion event.

In `@src/server/config.cpp`:
- Around line 73-74: The code assigns
project["max_active_file"].value<int64_t>() directly to
config.project.max_active_file via static_cast<std::size_t> which will wrap
negative inputs; update the handling around the call that reads
"max_active_file" so you first check that the optional<int64_t> (from
value<int64_t>()) has a value and that *v >= 0 before casting, and if the value
is negative or missing, either set a safe default or return/log an error;
specifically modify the block referencing project["max_active_file"], the local
variable v (result of value<int64_t>()), and the assignment to
config.project.max_active_file to perform the non-negative validation and error/
default handling.

In `@src/server/index_scheduler.cpp`:
- Around line 102-110: IndexScheduler currently only logs failures after
co_await pool_.send_stateless(params) and never updates the in-memory index, so
successful background indexing doesn't become visible; after confirming
result.has_value() and result->success, persist the returned index data into
project_index_ (e.g., merge or replace via the appropriate ProjectIndex API) so
newly indexed symbols are available to cross-file queries, taking any required
locking/atomic update into account; locate this logic around
pool_.send_stateless and the result/result->success handling and add the
persistence step to project_index_.
- Around line 91-102: The indexing request only sets worker::IndexParams
params.file while pcm_vec, directory and arguments are left out, so
StatelessWorker::on_index cannot replay the compile context; update the code
that builds params (worker::IndexParams) to assign the compiled module list and
command-line bits: copy pcm_vec into params (e.g. params.pcms or equivalent
field), set params.directory from the compile_graph_.get_pch/get_pcms context or
path's directory, and populate params.arguments with the same arguments produced
by compile_deps() so the worker receives the full compile context for replaying
the command line and module artifacts.

In `@src/server/memory_monitor.cpp`:
- Around line 41-44: Replace the hardcoded 4096 multiplier when computing memory
from "pages" by querying the system page size with sysconf(_SC_PAGESIZE); in
memory_monitor.cpp, include <unistd.h> if needed, call sysconf to get page_size
(handle a -1 fallback if syscall fails) and multiply pages by the returned
page_size (cast to std::size_t) before returning; update the return expression
that currently uses "pages * 4096" to use the queried page_size instead.

In `@src/server/options.cpp`:
- Around line 50-89: The CLI currently ignores unknown flags and omits the
--workers switch from help; update the argument parsing loop (the for loop and
extract_value lambda) so that unrecognized arguments produce a clear
error/warning (e.g., LOG_ERROR or LOG_WARN and exit or set a failure flag)
instead of silently falling back to defaults, and add the --workers description
to the help output alongside the other options; specifically modify the help
printing block and the final else branch of the parsing chain to report unknown
flags (referencing extract_value, opts.enable_workers, and Mode handling) so
typos and unsupported flags are surfaced to the user.
- Around line 83-86: The current numeric parsing for --port and
--worker-memory-limit is unsafe; replace the std::atoi/std::stoull usage by
calling std::from_chars on the extracted string (from extract_value) for each
option, verify the parse succeeded (ec == std::errc()) and that the pointer
reached the end of the string to reject trailing garbage, for --port
additionally check the parsed integer is in the 1–65535 range before assigning
to opts.port, and for --worker_memory_limit parse into an unsigned long long but
first reject any leading '-' and ensure full consumption; on any parse or range
failure, handle it gracefully (e.g., log a descriptive error and abort startup
or set a safe default and return an error) rather than letting exceptions or
reinterpretation of negative values occur.

In `@src/server/path_pool.h`:
- Around line 3-9: The header uses std::copy (transitively relied upon) but
doesn't include <algorithm>, so add `#include` <algorithm> to this header (place
it with the other standard includes near the top) to make the header
self-contained and ensure std::copy is declared for any translation unit that
includes path_pool.h.

In `@src/server/server.cpp`:
- Around line 825-829: The Server::path_to_uri function naively prepends
"file://" and fails to percent-encode special characters and platform-specific
separators; update it to produce a valid file URI by normalizing path separators
(convert backslashes to '/'), ensuring the proper leading slash for Windows
drive letters, and percent-encoding reserved characters (spaces, non-ASCII,
percent, ?, #, etc.) per RFC 3986; implement or call a utility encoder (e.g., a
percentEncode function) and use it inside Server::path_to_uri so the returned
string is a correctly escaped file:// URI for all platforms.

In `@src/server/stateless_worker.cpp`:
- Around line 220-225: The code currently hardcodes
worker::IndexResult.result.success = true after calling compile(compile_params)
and does not populate the TU index; change this to determine success from the
compile(...) return (e.g., inspect the returned unit for errors or a status
field) and set result.success accordingly, populate result.TUIndex (or the
appropriate TU container) from the compilation unit when indexing is
implemented, and on compile failure set result.success = false and include
error/diagnostic info so callers can retry; update references around compile,
compile_params, worker::IndexResult, result.success and the TU index population
to reflect these checks and avoid reporting success for empty/failed parses.

In `@src/server/worker_pool.cpp`:
- Around line 52-63: The pool shutdown currently closes worker pipes but
monitor_worker unconditionally calls restart_worker, causing workers to be
respawned; add a shutdown flag (e.g., WorkerPool::shutting_down as an
atomic<bool>) set to true at the start of WorkerPool::stop() before closing
outputs, and update monitor_worker (and any other place that currently calls
restart_worker on exit) to check that flag and skip calling restart_worker when
shutting_down is true so workers drain instead of being restarted; ensure the
flag is used in both stateless and stateful monitoring paths and is thread-safe
(atomic) so no races occur.
- Around line 178-208: The assign_worker implementation can assign index 0 when
stateful.workers is empty; add an explicit check at the top of
WorkerPool::assign_worker for stateful.workers.empty() and handle it (e.g.,
throw a std::runtime_error or return an invalid sentinel like SIZE_MAX) instead
of proceeding to compute best_idx; this prevents inserting
stateful.owner[path_id] with a non-existent worker index and avoids later
out-of-bounds access when dereferencing stateful.workers. Ensure the chosen
behavior is consistent with callers (adjust caller handling if you choose to
throw) and reference stateful.workers, stateful.owner, stateful.owner_lru, and
stateful.owner_lru_index when making the change.

---

Nitpick comments:
In `@cmake/package.cmake`:
- Around line 47-59: The FetchContent_Declare call for eventide currently uses
GIT_TAG main which makes builds non-reproducible; update the
FetchContent_Declare entry for the eventide target to pin GIT_TAG to a specific
release tag or a commit hash (replace "main" with a stable tag or commit) so the
dependency is fixed, and keep the rest of the ET_* option toggles and the
FetchContent_MakeAvailable(eventide ...) invocation unchanged.

In `@docs/agent/implement.md`:
- Line 567: The scheduled collect_worker_log task started via
loop.schedule(std::move(spawn.stderr_pipe), worker_id) can become orphaned when
a worker is restarted; add a cancellable task handle (e.g., et::task_handle
log_task) to the WorkerProcess struct and store the handle returned by
loop.schedule so you can call worker.log_task.cancel() (or equivalent) before
replacing the WorkerProcess in restart_worker and then reassign the new log_task
from the newly spawned worker (symbols: collect_worker_log, loop.schedule,
WorkerProcess, log_task, restart_worker, spawn_worker).

In `@src/server/config.cpp`:
- Around line 32-35: The handling of var_name == "version" and "llvm_version"
uses hardcoded strings via result.append("0.1.0") and result.append("21");
replace those with build-generated macros or constants (e.g. PROJECT_VERSION or
LLVM_VERSION_STRING) produced by CMake/configure_file and exposed via a version
header; update the code to append the macro/constant (e.g.
result.append(PROJECT_VERSION)) and ensure CMake generates and installs the
header or defines the macros (e.g. add configure_file or add_definitions) so the
version and llvm_version remain in sync with the build.

In `@src/server/memory_monitor.cpp`:
- Around line 51-62: MemoryMonitor::run is a placeholder that only logs instead
of performing memory checks or integrating with WorkerPool PIDs; update the
function by adding a clear TODO comment with an issue/ticket reference (or
create one) and a short note on required work (expose WorkerPool PIDs and query
per-worker memory metrics), and if helpful add a minimal assertion or
metric-emission stub (e.g., emit a "not implemented" metric) so the placeholder
is discoverable at runtime; reference MemoryMonitor::run, WorkerPool, and the
periodic sleep loop when adding the TODO and linking the issue.

In `@src/server/path_pool.h`:
- Around line 23-30: The code currently makes a second heap copy of the interned
path by allocating via allocator.Allocate and saving that into paths; instead
reuse the stable key storage inside the llvm::StringMap entry returned from
cache.try_emplace: after insertion, obtain the map-stored key (via the entry's
getKey() accessor, e.g. it->first.getKey() or it->first->getKey() depending on
your StringMap iterator type), construct an llvm::StringRef from that key and
push it into paths, update it->second accordingly, and remove the
allocator.Allocate copy and std::copy usage so you no longer duplicate the
string storage.

In `@src/server/progress.h`:
- Around line 33-38: The ProgressReporter class initializes next_token_ to 0
per-instance, causing token reuse across multiple ProgressReporter instances and
collisions over the same peer; change the design to use a shared token generator
instead of the instance member next_token_. Move token generation to a peer- or
process-scoped source (for example attach a token counter to et::ipc::JsonPeer
or use a shared std::atomic<std::uint32_t> / singleton generator) and replace
uses of ProgressReporter::next_token_ with calls to that shared getNextToken()
so tokens are globally unique across peer_ and processes.

In `@src/server/server.cpp`:
- Around line 720-772: resolve_module_deps currently does an O(N) scan of
cdb.files() each time a module name is missing from module_name_cache (see
resolve_module_deps, module_name_cache, cdb.files(), scan), which is expensive
for large projects; fix it by populating module_name_cache once (either at
Server startup or lazily on first miss) by iterating cdb.files() a single time
to map file_scan.module_name -> path_id (and mark interface units), then change
resolve_module_deps to consult that prebuilt map and only fall back to a
targeted scan if necessary; ensure compile_graph.register_unit logic remains the
same when inserting cached entries.
- Around line 672-692: The block in the non-worker build branch around the
compile path (uses make_compile_params, compile, documents.find/it3,
doc3.generation, feature::diagnostics, publish_diagnostics, update_index) has
inconsistent indentation for the statements between the if(doc3.generation !=
gen) check and the closing brace; reformat so all statements (auto diags = ...,
doc3.unit = ..., publish_diagnostics(...), and the if(doc3.unit)
update_index(doc3) block) are indented consistently with the surrounding
else-block and aligned with the opening brace of that scope, and ensure the
closing braces line up with the corresponding opening braces.

In `@src/server/worker_pool.h`:
- Around line 37-51: The round-robin counter in send_stateless (stateless.next)
is incremented unboundedly and can overflow; change the update so it does not
grow without bound—e.g., compute the worker index with stateless.next %
stateless.workers.size() but update stateless.next in a bounded way (set
stateless.next = (stateless.next + 1) % stateless.workers.size() or reset to
stateless.next % stateless.workers.size() when it exceeds a threshold) so the
counter is kept small and avoids overflow while preserving round-robin
semantics; apply this change in the send_stateless function where stateless.next
is read/updated.

In `@tests/integration/test_cross_file.py`:
- Around line 64-69: Update the capability assertions in
tests/integration/test_cross_file.py to ensure each capability key exists and is
not False instead of strict boolean checks or mere key-presence: for
result["capabilities"] (caps) assert "definitionProvider" in caps and
caps["definitionProvider"] is not False, assert "referencesProvider" in caps and
caps["referencesProvider"] is not False, and similarly assert
"callHierarchyProvider" in caps and caps["callHierarchyProvider"] is not False,
"typeHierarchyProvider" in caps and caps["typeHierarchyProvider"] is not False,
and "workspaceSymbolProvider" in caps and caps["workspaceSymbolProvider"] is not
False.
- Around line 7-18: test_go_to_definition and the related workspace symbol tests
are written as smoke tests that allow None/empty results; change them into
behavior checks that assert cross-file resolution succeeds by using a symbol
defined in another file. Update test_go_to_definition to open both source files
(e.g., main.cpp and the file that defines the target symbol), call
client.go_to_definition("main.cpp", 2, 4) and assert the returned
Location/target URI and range point to the other file and expected symbol span;
similarly, modify the test_workspace_symbol case (lines ~49-56) to call
client.workspace_symbol with the cross-file symbol name and assert the returned
symbol entries contain the correct name and URI rather than accepting an empty
list. Refer to functions client.go_to_definition and client.workspace_symbol to
locate and change the assertions.
- Around line 9-11: Replace the fixed await asyncio.sleep(5) calls with a
readiness wait that polls for the actual condition the test needs (e.g. the
symbol/token/result to appear) or use an index-ready/progress signal exposed by
the test fixture; specifically, after calling client.initialize(...) and
client.did_open("main.cpp") replace the blind sleeps with a loop or helper like
wait_for_condition/await_until that repeatedly queries the language server (e.g.
a workspace/symbol or documentSymbol request, diagnostics, or a custom
client.is_index_ready()/fixture.index_ready_event) until the expected
symbol/token is present or a timeout is reached; apply the same change for the
other occurrences of await asyncio.sleep(5) in this test so tests become
deterministic and CI-friendly.