bugfix: Initialize definitionIndexStrategy when initializing

[tgodzik]

Previously, we would do it after initialized at which point the indexing has already happened. Now, we index according to the strategy the user wants.

The other option is to handle the change, but if someone uses non default, they will always get double indexing.

CC @olafurpg though it doesn't really change anything for feature flags.

A side note is that I realized that even though the classpath indexing is faster it will miss features such as type member completions etc. This can easily be chosen by the user, for larger codebase they could turn on classpath, but for smaller keeup using sources.

Summary by CodeRabbit

• New Features

  • Added support for configuring the definition index strategy through initialization options.
  • Feature flags can provide the definition index strategy when no initialization option is set.
  • A default strategy is applied when no custom setting is provided.

• Improvements

  • Definition indexing now uses the resolved client configuration consistently across workspace, dependency, and classpath indexing.
  • Removed the definition index strategy from user configuration in favor of centralized client settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c42f899-81e3-4ed9-9bd5-ccf24ba36930

📥 Commits

Reviewing files that changed from the base of the PR and between 52624ed and a1d2078.

📒 Files selected for processing (1)

• metals-mcp/src/main/scala/scala/meta/metals/McpMain.scala

🚧 Files skipped from review as they are similar to previous changes (1)

• metals-mcp/src/main/scala/scala/meta/metals/McpMain.scala

---

📝 Walkthrough

Walkthrough

The definition index strategy now comes from initialization options or feature flags through ClientConfiguration. UserConfiguration no longer stores the strategy. Indexing services and tests use the new configuration path.

Changes

Definition index strategy

Layer / File(s)	Summary
Strategy contract and resolution metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala, metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala, metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala, metals/src/main/scala/scala/meta/internal/metals/Configs.scala	Initialization options store the optional strategy. ClientConfiguration resolves it from initialization options or feature flags. UserConfiguration no longer stores or serializes it.
Configuration wiring and indexing metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala, metals-mcp/src/main/scala/scala/meta/..., metals/src/main/scala/scala/meta/internal/metals/Indexer.scala, metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala	Service entry points pass feature flags to ClientConfiguration. Indexing services read the resolved strategy from ClientConfiguration.
Test configuration migration tests/unit/src/test/scala/tests/CompletionLspSuite.scala, tests/unit/src/test/scala/tests/UserConfigurationSuite.scala	Tests configure the strategy through initialization options and update client configuration construction and serialized configuration expectations.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ClientConfiguration
  participant FeatureFlagProvider
  participant Indexer
  Client->>ClientConfiguration: send initialization options
  ClientConfiguration->>FeatureFlagProvider: resolve fallback strategy
  FeatureFlagProvider-->>ClientConfiguration: return strategy
  ClientConfiguration->>Indexer: provide definitionIndexStrategy()
  Indexer->>Indexer: apply strategy to dependency indexing

Loading

Possibly related PRs

• scalameta/metals#8206: Changes related MCP configuration through StandaloneMcpService, McpMain, and ClientConfiguration.

Suggested reviewers: zielinsky

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: initializing definitionIndexStrategy during client initialization.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[tgodzik]

Added scalameta/metals-vscode#1947 to show how this would work. I want to gather up all the similar options together as they heavily influence the performance and behaviour

[coderabbitai]

🧹 Nitpick comments (1)

metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala (1)

1360-1366: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename ProtoOutlineProviderConfig.fromConfigOrFeatureFlag now that it no longer uses feature flags.

The method name fromConfigOrFeatureFlag still implies feature-flag resolution, but the signature in Configs.scala no longer accepts a FeatureFlagProvider and the None case only returns ProtoOutlineProviderConfig.default. Every other fromConfigOrFeatureFlag sibling method (WorkspaceSymbolProviderConfig, DefinitionProviderConfig, ReferenceProviderConfig, and so on) still resolves a feature flag in the None case, so this method now breaks that naming convention. Rename it to fromConfig for clarity.

🤖 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 `@metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala`
around lines 1360 - 1366, Rename
ProtoOutlineProviderConfig.fromConfigOrFeatureFlag to fromConfig in its
definition and update the invocation inside UserConfiguration’s
protoOutlineProvider initialization; preserve the existing parsing and default
behavior.

🤖 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 `@metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala`:
- Around line 1360-1366: Rename
ProtoOutlineProviderConfig.fromConfigOrFeatureFlag to fromConfig in its
definition and update the invocation inside UserConfiguration’s
protoOutlineProvider initialization; preserve the existing parsing and default
behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 34b47472-a0e5-438d-92df-bc1a0bdd99e6

📥 Commits

Reviewing files that changed from the base of the PR and between 79568e2 and 52624ed.

📒 Files selected for processing (13)

• metals-mcp/src/main/scala/scala/meta/internal/metals/mcp/StandaloneMcpService.scala
• metals-mcp/src/main/scala/scala/meta/metals/McpMain.scala
• metals/src/main/scala/scala/meta/internal/metals/ClientConfiguration.scala
• metals/src/main/scala/scala/meta/internal/metals/Configs.scala
• metals/src/main/scala/scala/meta/internal/metals/Indexer.scala
• metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala
• metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
• metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala
• metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala
• tests/unit/src/main/scala/tests/BaseLspSuite.scala
• tests/unit/src/test/scala/tests/CompletionLspSuite.scala
• tests/unit/src/test/scala/tests/ManualSuite.scala
• tests/unit/src/test/scala/tests/UserConfigurationSuite.scala

💤 Files with no reviewable changes (2)

• tests/unit/src/main/scala/tests/BaseLspSuite.scala
• tests/unit/src/test/scala/tests/ManualSuite.scala