feat(server): index-based declaration, implementation and typeDefinition

[16bit-ykiko]

Background

textDocument/declaration, implementation and typeDefinition were advertised in the initialize capabilities but their handlers returned null stubs; find-references worked but returned null for empty results and only merged definitions under includeDeclaration. This PR completes the index-based navigation set.

Implementation

• Two-hop target queries (Indexer::query_symbol_targets): resolve the cursor to a symbol, collect between-symbol relation targets (Implementation, TypeDefinition), then resolve each target hash to its definition location — the same machinery type hierarchy already uses. Relation directions are locked by unit tests: a base virtual method's Implementation relations point at its direct overrides; each level of a deep override chain navigates to its own overriders.
• declaration returns declarations plus the definition: inline-defined symbols have no separate Declaration relation, and navigating to the definition is what clients expect there.
• references: includeDeclaration now includes declarations as well as definitions; empty results return [].
• Closed documents are index-serveable: resolve_cursor previously bailed without a session, so every index query on a closed file returned empty — it now maps positions through the merged shard's own stored content. Navigation on closed documents works, and an empty result is a real answer ([]), never an error.

All three new methods are index-only by design: no worker fallback (go-to-definition's worker fallback already covers the fresh-buffer path that matters).

Recorded index gaps (deliberately not fixed here)

Locked by tests so future indexer fixes surface as intentional test updates:

• auto-deduced variables carry no TypeDefinition relation; alias-typed variables navigate to the using declaration instead of unwrapping.
• Functions carry no TypeDefinition relation for their return type.
• Background indexing skips open files, so a header included only by an open TU has no shard until that file is closed (sessions carry only the interested file's index).

Tests

• Unit: relation-direction locks (ImplementationDirection, TypeDefinitionTargets) on the index query harness.
• E2E on a new nav.h/nav.cpp fixture: cross-file declaration, inline-definition declaration, forward-declaration navigation, virtual override and pure-virtual implementation, chained-override direct-only semantics, typeDefinition, references with exact-set assertions for both includeDeclaration modes, and the open/closed empty-result contract.

Test plan

• RelWithDebInfo: 734 unit / 198 integration / 3 smoke — green
• Debug: 734 unit / 198 integration / 3 smoke — green
• pixi run format clean; 3 parallel review subagents (correctness / style / tests) — findings addressed

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an index-backed query_symbol_targets API to the Indexer for resolving symbol relations (implementation, type definition), wires it into LSP navigation handlers (declaration, implementation, type-definition, references) to return index-only results instead of null stubs, and adds corresponding test fixtures, unit/integration tests, and documentation updates.

Changes

Index-only navigation feature

Layer / File(s)	Summary
Indexer cursor resolution and query_symbol_targets src/server/compiler/indexer.cpp, src/server/compiler/indexer.h	Adjusts resolve_cursor fallback offset computation to use session or shard-derived line maps, and adds query_symbol_targets to resolve a cursor to relation targets and return their definition locations.
LSP navigation handlers wiring src/server/service/lsp_client.cpp	Adds query_targets_at lambda and reworks declaration/implementation/typeDefinition/references handlers to use index-backed queries, returning empty arrays instead of null.
Test fixtures for navigation scenarios tests/data/index_features/nav.h, tests/data/index_features/nav.cpp, tests/data/index_features/CMakeLists.txt	Adds Shape, Renderer/GLRenderer/DebugGLRenderer hierarchy, and helper functions used by navigation tests; wires nav.cpp into the build.
Test client navigation helpers tests/integration/utils/client.py	Adds declaration_at, implementation_at, type_definition_at async methods and related lsprotocol.types imports.
Integration tests for navigation behavior tests/integration/features/test_index.py	Adds locations_of helper and tests covering declaration, implementation, type-definition, references, override chains, forward declarations, and empty-result cases.
Unit tests and documentation updates tests/unit/index/index_query_tests.cpp, docs/en/features/navigation.md	Adds ImplementationDirection and TypeDefinitionTargets unit tests; updates navigation docs and changelog table to reflect completed index-based navigation features.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant LspClientHandler
  participant Indexer
  participant Shard

  Client->>LspClientHandler: textDocument/declaration or implementation or typeDefinition
  LspClientHandler->>LspClientHandler: query_targets_at(uri, position, kind)
  LspClientHandler->>Indexer: query_symbol_targets(path, position, kind, session)
  Indexer->>Indexer: resolve_cursor(position, session)
  Indexer->>Shard: to_offset(position) via session or shard content
  Indexer->>Indexer: collect target symbol hashes for RelationKind
  Indexer->>Indexer: resolve_symbol(target hash) per target
  Indexer-->>LspClientHandler: protocol::Location list
  LspClientHandler-->>Client: locations or empty array

Loading

Poem

A rabbit hops through symbol trees,
declarations found with practiced ease,
no more nulls where locations hide,
implementations chained with pride,
type definitions, tests in tow —
this warren's index-navigation glow. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: index-based declaration, implementation, and typeDefinition support on the server.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/goto-navigation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/integration/features/test_index.py (1)

380-391: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider asserting no duplicate locations.

This test only checks membership (in), so it wouldn't catch a duplicate ("nav.h", 6) entry if the declaration handler returns the record's definition site twice (see companion comment in lsp_client.cpp). Asserting len(locs) == len(set(lines)) or exact set equality would make this test also guard against that regression.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/features/test_index.py` around lines 380 - 391, The
declaration lookup test for test_goto_declaration_forward_declared only checks
that expected locations are present, so it can miss duplicate results from
client.declaration_at. Update the assertions in this test to validate uniqueness
as well, using the existing locs/lines data from locations_of so that a repeated
nav.h definition entry is caught, ideally by asserting exact set equality or by
comparing len(locs) to the number of unique lines.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/integration/features/test_index.py`:
- Around line 380-391: The declaration lookup test for
test_goto_declaration_forward_declared only checks that expected locations are
present, so it can miss duplicate results from client.declaration_at. Update the
assertions in this test to validate uniqueness as well, using the existing
locs/lines data from locations_of so that a repeated nav.h definition entry is
caught, ideally by asserting exact set equality or by comparing len(locs) to the
number of unique lines.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: feb5b762-7bed-4122-b24b-af0788bdefa9

📥 Commits

Reviewing files that changed from the base of the PR and between 27717f3 and 87343b5.

📒 Files selected for processing (10)

• docs/en/features/navigation.md
• src/server/compiler/indexer.cpp
• src/server/compiler/indexer.h
• src/server/service/lsp_client.cpp
• tests/data/index_features/CMakeLists.txt
• tests/data/index_features/nav.cpp
• tests/data/index_features/nav.h
• tests/integration/features/test_index.py
• tests/integration/utils/client.py
• tests/unit/index/index_query_tests.cpp