refactor: extract memory, Leiden and the LLM stack into standalone crates - #207
Merged
Merged
Conversation
added 7 commits
July 27, 2026 23:36
The long-term memory subsystem now lives in its own project (memory-rs), so remove it from codesearch entirely: the memory store, extraction, search, browse, the TUI Memory mode, the memory controller/handlers, and the domain model. Also remove the ring of code that existed only to feed memory: the dream scheduler, session discovery (Claude/OpenCode/Zed), transcript parsing, the interactive import picker, and the resource fetcher. Kept: channel extraction (a code-analysis feature independent of memory), InMemoryVectorRepository (the RAM-backed vector store behind --memory-storage), and everything else. Removed surfaces: the `memory` and `dream` CLI subcommands, the TUI Memory mode + import picker, the `/api/memory*`, `/api/sessions*`, and `/api/memory/dream*` REST endpoints, and the memory-related MCP tools (search_memory, list_memories, read_memory, add_memory_resource). The management API's OpenAPI document is pruned to match. BREAKING CHANGE: the memory/session/dream REST + MCP surfaces are gone; consumers must talk to memory-rs instead.
The Leiden clustering algorithm and the coupling-element analysis have been extracted into two standalone, domain-agnostic crates in the leiden-rs workspace. Add them as path dependencies ahead of the swap. petgraph was listed for "Leiden cluster detection" but is not actually used anywhere in the codebase, so remove it.
Delete the generic algorithm halves of cluster_detection, coupling_detection, and symbol_cluster_detection — the Leiden core (local moving, refinement, aggregation, resolution search, post-passes, the deterministic PRNG) and the coupling pipeline (fragility probe, min-cut/Dinic flow, candidate scoring, ablation verification, god-object detection, the façade split) — and call the standalone leiden / leiden-coupling crates instead. What stays is codesearch policy: the reference-kind edge weighting (kind_weight, composite_weight, build_file_leiden_graph), the façade-split configuration read from the environment, namespace qualification, cohesion and importance scoring, and the mapping from the crates' domain-free result types into codesearch's CouplingReport / CouplingElement. The crates' default configs reproduce the tuned constants exactly, so partition membership and coupling reports are unchanged — the cluster, coupling, and integration test suites pass as-is. The algorithm's own unit tests moved into the crates with the code.
The OpenAI-compatible chat/embeddings client and the GitHub Copilot backend have been extracted into standalone crates. Add them as path dependencies ahead of the swap.
Replace codesearch's hand-rolled OpenAI-compatible and GitHub Copilot
clients with the extracted crates, keeping codesearch's ChatClient /
EmbeddingService ports and DomainError as the boundary:
- OpenAiChatClient wraps openai_rs::OpenAiChatClient. This inverts to the
Responses API first (Chat Completions fallback, cached per client) and
no longer hardcodes temperature — so the adapter pins temperature=0.0
on every request to preserve codesearch's deterministic extraction.
- CopilotChatClient wires gh_copilot_rs::CopilotEndpoint to the same
OpenAI client via an unversioned, Copilot-headed transport; model
discovery goes through gh_copilot_rs::CopilotModelCatalog. The crate's
CopilotModel types are re-exported under their old names, so the picker
needs only an import change.
- OpenAiEmbedding / LmStudioEmbedding delegate the HTTP body to
openai_rs::OpenAiEmbeddingClient (batching + L2-normalise on by
default), keeping the 60s timeout and the dimension-mismatch warning.
- copilot login (CLI) uses gh_copilot_rs::{GitHubDeviceFlow, LoginUseCase};
serve-mode CopilotLoginService wraps gh_copilot_rs::LoginSession and
persists the token where it observes success. The LoginStatus JSON shape
is unchanged, so the management-API contract is preserved.
- Add From<OpenAiError>/From<CopilotError> for DomainError; delete the
superseded copilot_auth device-flow module.
Update AGENTS.md, README.md, and the feature/architecture docs to reflect that the long-term memory subsystem is gone (now its own project) and that the LLM and Leiden stacks are backed by the openai-rs / gh-copilot-rs / leiden-rs crates. Delete docs/features/memory.md and the memory endpoint tables.
…202) Cherry-picked from 7d1e800 onto the extract-crates branch. /api/clusters and /api/symbol-clusters return community membership only — no edge adjacency — so a client can't draw the community graph the `visualize` CLI renders. This returns the full GraphView (nodes + edges + communities) for one repository at the file (default) or symbol level, reusing the same graph_view() builders the visualize path drives. Also carries #202's fix for /api/channels: the `repository` filter was Option<Vec<String>>, which axum's default Query cannot deserialize from any query form, so the filter silently never bound. Applies cleanly over the leiden-rs migration; the management server tests pass. Needed by Hoplon's namespace community-graph view.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes the long-term memory subsystem (domain models, ChangesCodesearch Platform Reshaping
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as copilot_command
participant LoginUseCase
participant Service as CopilotLoginService
participant Session as LoginSession
participant Config as config.json
CLI->>LoginUseCase: begin()
LoginUseCase-->>CLI: device code
CLI->>LoginUseCase: wait_for_token()
Service->>Session: start()
Session-->>Service: status Pending
loop poll status
Service->>Session: status()
end
Session-->>Service: status Authorized
Service->>Config: write token (0600)
sequenceDiagram
participant Controller
participant BuildChatClientFor as build_chat_client_for
participant Config as CodesearchConfig
participant Endpoint as CopilotEndpoint/OpenAiChatClient
Controller->>BuildChatClientFor: usage, target, data_dir
BuildChatClientFor->>Config: load usages
Config-->>BuildChatClientFor: UsageBinding (endpoint, model)
BuildChatClientFor->>Endpoint: construct with binding or defaults
Endpoint-->>Controller: ChatClient
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/features/serve-and-management-api.md (1)
96-110: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdd the documented LLM routes to the OpenAPI contract.
This section introduces
/api/llm/endpoints,/api/llm/active, and/api/llm/models, butdocs/management-api.openapi.jsondocuments none of them. Since the checked-in OpenAPI document is the advertised contract, clients cannot discover these APIs until the paths and schemas are added.🤖 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 `@docs/features/serve-and-management-api.md` around lines 96 - 110, Update the checked-in OpenAPI contract in docs/management-api.openapi.json to document the LLM management routes described in the “LLM backend management” section: GET /api/llm/endpoints, PUT /api/llm/endpoints/{name}, POST /api/llm/active, and GET /api/llm/models, including request/response schemas, parameters, and masked/write-only api_key behavior consistent with the documentation.docs/management-api.openapi.json (1)
1259-1269: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign
ExplainStreamRequest.llmwith the supported Copilot backend.The management API supports
openai,anthropic, andcopilot, andExplainStreamRequest.llmis documented for the explain stream, but its enum omitscopilot. Addcopilotto the enum or remove the conflicting documentation for Copilot as a selectable stream backend.🤖 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 `@docs/management-api.openapi.json` around lines 1259 - 1269, Update the ExplainStreamRequest llm schema enum to include the supported copilot backend alongside openai, anthropic, and null, while preserving its existing type and description.src/connector/adapter/lm_studio_embedding.rs (1)
110-139: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
embed_chunkslost batching and can silently drop embeddings in both adapters. Removing theBATCH_SIZEloop sends every chunk in a single request under a 60s timeout, andchunks.iter().zip(vectors)yields onlymin(len)pairs, so a short response drops the tail chunks with no error.
src/connector/adapter/lm_studio_embedding.rs#L110-L139: add an explicitvectors.len() != chunks.len()error and confirm the crate batches internally (or reinstate a local batch size).src/connector/adapter/openai_embedding.rs#L111-L140: apply the same length check and batching decision.🤖 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 `@src/connector/adapter/lm_studio_embedding.rs` around lines 110 - 139, The embed_chunks implementations in src/connector/adapter/lm_studio_embedding.rs lines 110-139 and src/connector/adapter/openai_embedding.rs lines 111-140 must validate that embed_texts returns exactly one vector per input chunk before zipping, returning an appropriate DomainError on mismatch instead of silently dropping chunks. Also verify whether the underlying crate batches requests internally; if not, reinstate local BATCH_SIZE batching in both embed_chunks methods while preserving the existing embedding construction.src/connector/adapter/openai_chat_client.rs (1)
154-165: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a fallback route for unsupported schema responses.
complete_jsonnow always sendsresponse_format: { type: "json_schema" }, so servers in the JSON-extraction paths that previously worked with a plain completion fallback when 4xxed will now fail immediately. Keep or migrate theopenai-rsfallback for unsupported schema/format requests.🤖 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 `@src/connector/adapter/openai_chat_client.rs` around lines 154 - 165, Update OpenAiChatClient::complete_json to retain a fallback when the schema-based request is rejected as unsupported, retrying through the existing openai-rs/plain JSON-extraction completion route for the relevant 4xx schema or response-format errors. Preserve the schema request as the primary path and return non-unsupported errors unchanged.
🧹 Nitpick comments (6)
src/application/use_cases/coupling_detection.rs (1)
20-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffApplication layer now depends on a third-party crate.
use leiden_coupling::…places an external dependency directly in the Application layer. Consider defining a port trait insrc/application/interfaces/and keeping the crate call in a Connector adapter, which would also makedetectunit-testable without the algorithm crate. Fine to defer, but worth tracking.As per coding guidelines: "Keep application code dependent only on the Domain layer; define port traits in
src/application/interfaces/."Also applies to: 79-93
🤖 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 `@src/application/use_cases/coupling_detection.rs` around lines 20 - 24, Move the leiden_coupling integration out of the application layer: define an algorithm port trait under application/interfaces, update detect to depend on that abstraction, and implement the trait in a Connector adapter that owns analyze, CommunityCoupling, Coupler, and CouplerKind usage. Preserve detect’s existing behavior while enabling unit tests without the third-party crate.Source: Coding guidelines
src/connector/adapter/management/copilot_login.rs (3)
87-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMisleading log message. The token was not saved here; "token saved-but-failed" reads as the opposite.
🪵 Clarify the message
- warn!("copilot login: token saved-but-failed: {e}"); + warn!("copilot login: authorized but failed to persist token: {e}");🤖 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 `@src/connector/adapter/management/copilot_login.rs` around lines 87 - 89, The warning in the copilot login flow around persist_token should not claim the token was saved when persistence failed. Update the warn! message in the persist_token error branch to accurately state that saving the token failed, while retaining the error detail.
83-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName the poll interval.
♻️ Extract a named constant
+/// How often the background task re-checks the device-flow session status. +const POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(500); +- tokio::time::sleep(std::time::Duration::from_millis(500)).await; + tokio::time::sleep(POLL_INTERVAL).await;As per coding guidelines: "Define named constants for numeric values that are not immediately obvious".
🤖 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 `@src/connector/adapter/management/copilot_login.rs` at line 83, Replace the inline 500-millisecond duration in the polling loop with a named constant defined near the relevant login or polling logic, and use that constant in the tokio sleep call. Choose a descriptive name that communicates it is the poll interval while preserving the existing 500 ms behavior.Source: Coding guidelines
14-19: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe
LoginStatuswire contract is now owned by an external crate with no test guarding it.The docs assert the serialized shape is unchanged for the native app, but the in-file test that validated that contract was dropped along with the local
LoginStatus. A futuregh-copilot-rsbump can silently change the JSON forGET /api/llm/copilot/login. Worth re-adding a small serialization assertion overgh_copilot_rs::LoginStatus(pending/authorized/failed variants).🤖 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 `@src/connector/adapter/management/copilot_login.rs` around lines 14 - 19, Add a focused serialization test for gh_copilot_rs::LoginStatus covering pending, authorized, and failed variants, asserting their JSON shapes match the existing native HTTP contract. Place it with the Copilot login tests and keep the production wrapper and status behavior unchanged.src/connector/adapter/management/handlers/graph_view.rs (1)
59-85: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftCPU-bound graph build/aggregation runs on the request task.
graph_view()(Leiden community detection) plusaggregate()are CPU-heavy and synchronous once the data is loaded; on a large repository this stalls the runtime worker serving other requests. Consider moving the CPU phase totokio::task::spawn_blocking(matching how the config reads elsewhere are offloaded), and note that a concurrency guard or response cache would help since each request recomputes the whole graph.As per coding guidelines: "Use async/await for I/O and wrap blocking calls in
tokio::task::spawn_blocking".🤖 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 `@src/connector/adapter/management/handlers/graph_view.rs` around lines 59 - 85, Move the CPU-bound graph construction and aggregation out of the request task by wrapping the GraphView selection via graph_view() and subsequent aggregate() decision in tokio::task::spawn_blocking. Preserve the GraphViewLevel::File and GraphViewLevel::Symbol behavior, propagate join and computation errors appropriately, and keep the existing DEFAULT_NODE_LIMIT and explicit aggregate override semantics.Source: Coding guidelines
src/connector/adapter/management/server.rs (1)
191-191: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the explicit
port, publicfield declarations.
portandpublicare non-skipped arguments and are normally recorded by#[tracing::instrument]; declaring them unsaved infields(...)adds emptytracing::field::Emptyfields.♻️ Drop the redundant fields
-#[tracing::instrument(skip(container), fields(port, public))] +#[tracing::instrument(skip(container))]🤖 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 `@src/connector/adapter/management/server.rs` at line 191, Update the tracing attribute on the affected function by removing the explicit fields(port, public) declaration, while retaining skip(container) so the non-skipped port and public arguments are recorded automatically.
🤖 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.
Inline comments:
In `@Cargo.toml`:
- Around line 69-77: Replace the sibling-directory path dependencies for leiden,
leiden-coupling, openai-rs, and gh-copilot-rs in the dependency declarations
with installable, reproducible sources such as published crates or pinned git
revisions. Ensure clean checkouts, CI, and package publishing resolve these
dependencies without requiring sibling repositories.
In `@docs/architecture/overview.md`:
- Around line 86-91: Remove all remaining memory-related references from
docs/architecture/overview.md, including mentions of MemoryItem,
MemoryRepository, and the background memory-dream scheduler. Update the affected
architecture sections to describe only the currently supported APIs and
components, while preserving the surrounding documentation structure.
In `@README.md`:
- Around line 267-270: Update the MCP tool documentation so the stated count
matches the enumerated tools: either add the four omitted tool names to the list
or change “20 tools” to the actual 16-tool count, keeping the heading and list
consistent.
In `@src/connector/adapter/copilot_chat_client.rs`:
- Around line 50-66: Update CopilotChatClient::new so a missing or empty model
is not passed as an empty model_id to OpenAiChatClient; either use the
established named default model constant or return DomainError::invalid_input
with a clear “no Copilot model selected” error before constructing the
transport/client.
In `@src/connector/adapter/openai_chat_client.rs`:
- Around line 134-139: Update the adapter struct and its constructors to retain
the configured Endpoint, API key, and timeout rather than only base_url. In
list_models, reuse the stored authenticated, timed endpoint, with an
Endpoint::new(base_url) fallback for the with_transport path, so existing
transport behavior remains unchanged.
In `@src/connector/adapter/openai_embedding.rs`:
- Around line 49-52: The OpenAI and LM Studio embedding constructors must
propagate client-build failures instead of panicking. In
src/connector/adapter/openai_embedding.rs lines 49-52 and
src/connector/adapter/lm_studio_embedding.rs lines 48-52, change each new()
constructor to return Result<Self, DomainError>, replace expect-based client
creation with ?, and update their containers and call sites to handle the
fallible constructors.
In `@src/domain/error.rs`:
- Around line 75-89: Remove the openai_rs::OpenAiError and
gh_copilot_rs::CopilotError From implementations from DomainError in
src/domain/error.rs, keeping the domain layer free of connector dependencies.
Add equivalent conversion helpers in the relevant connector adapter modules,
preserving source context rather than reducing errors to only to_string(), and
update adapter call sites to use those mappings.
In `@tests/management_server_tests.rs`:
- Around line 258-286: Strengthen
channels_endpoint_accepts_comma_separated_repository_filter by indexing a second
repository with distinct channel data, then assert each repository filter
response excludes channels from the unrelated repository. Keep the existing
status and response-shape checks, and ensure the assertions would fail if the
handler ignored the repository filter.
---
Outside diff comments:
In `@docs/features/serve-and-management-api.md`:
- Around line 96-110: Update the checked-in OpenAPI contract in
docs/management-api.openapi.json to document the LLM management routes described
in the “LLM backend management” section: GET /api/llm/endpoints, PUT
/api/llm/endpoints/{name}, POST /api/llm/active, and GET /api/llm/models,
including request/response schemas, parameters, and masked/write-only api_key
behavior consistent with the documentation.
In `@docs/management-api.openapi.json`:
- Around line 1259-1269: Update the ExplainStreamRequest llm schema enum to
include the supported copilot backend alongside openai, anthropic, and null,
while preserving its existing type and description.
In `@src/connector/adapter/lm_studio_embedding.rs`:
- Around line 110-139: The embed_chunks implementations in
src/connector/adapter/lm_studio_embedding.rs lines 110-139 and
src/connector/adapter/openai_embedding.rs lines 111-140 must validate that
embed_texts returns exactly one vector per input chunk before zipping, returning
an appropriate DomainError on mismatch instead of silently dropping chunks. Also
verify whether the underlying crate batches requests internally; if not,
reinstate local BATCH_SIZE batching in both embed_chunks methods while
preserving the existing embedding construction.
In `@src/connector/adapter/openai_chat_client.rs`:
- Around line 154-165: Update OpenAiChatClient::complete_json to retain a
fallback when the schema-based request is rejected as unsupported, retrying
through the existing openai-rs/plain JSON-extraction completion route for the
relevant 4xx schema or response-format errors. Preserve the schema request as
the primary path and return non-unsupported errors unchanged.
---
Nitpick comments:
In `@src/application/use_cases/coupling_detection.rs`:
- Around line 20-24: Move the leiden_coupling integration out of the application
layer: define an algorithm port trait under application/interfaces, update
detect to depend on that abstraction, and implement the trait in a Connector
adapter that owns analyze, CommunityCoupling, Coupler, and CouplerKind usage.
Preserve detect’s existing behavior while enabling unit tests without the
third-party crate.
In `@src/connector/adapter/management/copilot_login.rs`:
- Around line 87-89: The warning in the copilot login flow around persist_token
should not claim the token was saved when persistence failed. Update the warn!
message in the persist_token error branch to accurately state that saving the
token failed, while retaining the error detail.
- Line 83: Replace the inline 500-millisecond duration in the polling loop with
a named constant defined near the relevant login or polling logic, and use that
constant in the tokio sleep call. Choose a descriptive name that communicates it
is the poll interval while preserving the existing 500 ms behavior.
- Around line 14-19: Add a focused serialization test for
gh_copilot_rs::LoginStatus covering pending, authorized, and failed variants,
asserting their JSON shapes match the existing native HTTP contract. Place it
with the Copilot login tests and keep the production wrapper and status behavior
unchanged.
In `@src/connector/adapter/management/handlers/graph_view.rs`:
- Around line 59-85: Move the CPU-bound graph construction and aggregation out
of the request task by wrapping the GraphView selection via graph_view() and
subsequent aggregate() decision in tokio::task::spawn_blocking. Preserve the
GraphViewLevel::File and GraphViewLevel::Symbol behavior, propagate join and
computation errors appropriately, and keep the existing DEFAULT_NODE_LIMIT and
explicit aggregate override semantics.
In `@src/connector/adapter/management/server.rs`:
- Line 191: Update the tracing attribute on the affected function by removing
the explicit fields(port, public) declaration, while retaining skip(container)
so the non-skipped port and public arguments are recorded automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b1745675-0901-49e0-8021-78ca71f7b3c0
📒 Files selected for processing (76)
AGENTS.mdCargo.tomlREADME.mddocs/README.mddocs/architecture/overview.mddocs/features/memory.mddocs/features/serve-and-management-api.mddocs/management-api.openapi.jsonsrc/application/interfaces/memory_repository.rssrc/application/interfaces/mod.rssrc/application/interfaces/session_discovery.rssrc/application/use_cases/cluster_detection.rssrc/application/use_cases/coupling_detection.rssrc/application/use_cases/import_session.rssrc/application/use_cases/memory_browse.rssrc/application/use_cases/memory_dream.rssrc/application/use_cases/memory_dream_prompt.rssrc/application/use_cases/memory_extraction.rssrc/application/use_cases/memory_extraction_prompt.rssrc/application/use_cases/memory_search.rssrc/application/use_cases/memory_summary.rssrc/application/use_cases/memory_support.rssrc/application/use_cases/mod.rssrc/application/use_cases/symbol_cluster_detection.rssrc/cli/mod.rssrc/connector/adapter/claude_transcript.rssrc/connector/adapter/codesearch_config.rssrc/connector/adapter/copilot_auth.rssrc/connector/adapter/copilot_chat_client.rssrc/connector/adapter/duckdb_memory_repository.rssrc/connector/adapter/lm_studio_embedding.rssrc/connector/adapter/management/copilot_login.rssrc/connector/adapter/management/dream.rssrc/connector/adapter/management/handlers/channels.rssrc/connector/adapter/management/handlers/graph_view.rssrc/connector/adapter/management/handlers/llm.rssrc/connector/adapter/management/handlers/memory.rssrc/connector/adapter/management/handlers/mod.rssrc/connector/adapter/management/handlers/sessions.rssrc/connector/adapter/management/mod.rssrc/connector/adapter/management/server.rssrc/connector/adapter/management/session_import.rssrc/connector/adapter/mcp/server.rssrc/connector/adapter/mod.rssrc/connector/adapter/openai_chat_client.rssrc/connector/adapter/openai_embedding.rssrc/connector/adapter/resource_fetch.rssrc/connector/adapter/session_discovery/claude.rssrc/connector/adapter/session_discovery/mod.rssrc/connector/adapter/session_discovery/opencode.rssrc/connector/adapter/session_discovery/zed.rssrc/connector/api/container.rssrc/connector/api/controller/memory_controller.rssrc/connector/api/controller/mod.rssrc/connector/api/controller/stats_controller.rssrc/connector/api/copilot_command.rssrc/connector/api/mod.rssrc/connector/api/repo_resolver.rssrc/connector/api/router.rssrc/domain/error.rssrc/domain/models/discovered_session.rssrc/domain/models/memory.rssrc/domain/models/mod.rssrc/lib.rssrc/main.rssrc/tui/app.rssrc/tui/cache.rssrc/tui/event.rssrc/tui/import_picker.rssrc/tui/mod.rssrc/tui/state.rssrc/tui/views/memory.rssrc/tui/views/mod.rssrc/tui/widgets/input_bar.rstests/management_server_tests.rstests/memory_tests.rs
💤 Files with no reviewable changes (43)
- docs/features/memory.md
- docs/README.md
- src/connector/adapter/management/mod.rs
- src/tui/views/memory.rs
- src/domain/models/discovered_session.rs
- src/application/use_cases/memory_summary.rs
- src/connector/adapter/claude_transcript.rs
- src/connector/api/controller/memory_controller.rs
- tests/memory_tests.rs
- src/application/interfaces/memory_repository.rs
- src/tui/import_picker.rs
- src/connector/adapter/duckdb_memory_repository.rs
- src/connector/adapter/session_discovery/zed.rs
- src/application/use_cases/memory_search.rs
- src/connector/adapter/session_discovery/opencode.rs
- src/connector/adapter/session_discovery/claude.rs
- src/connector/adapter/management/session_import.rs
- src/tui/views/mod.rs
- src/tui/mod.rs
- src/application/use_cases/memory_dream_prompt.rs
- src/application/use_cases/memory_support.rs
- src/domain/models/memory.rs
- src/connector/adapter/copilot_auth.rs
- src/application/interfaces/session_discovery.rs
- src/connector/adapter/session_discovery/mod.rs
- src/application/use_cases/memory_extraction_prompt.rs
- src/connector/adapter/management/handlers/sessions.rs
- src/application/use_cases/memory_browse.rs
- src/application/use_cases/memory_dream.rs
- src/application/interfaces/mod.rs
- src/application/use_cases/import_session.rs
- src/tui/event.rs
- src/connector/adapter/management/dream.rs
- src/application/use_cases/mod.rs
- src/connector/adapter/management/handlers/memory.rs
- src/domain/models/mod.rs
- src/application/use_cases/memory_extraction.rs
- src/connector/adapter/resource_fetch.rs
- src/connector/adapter/codesearch_config.rs
- src/connector/adapter/mod.rs
- src/connector/api/repo_resolver.rs
- src/tui/state.rs
- src/tui/cache.rs
added 3 commits
July 28, 2026 09:56
…ent API Cherry-picks 2716cf4 and 9631bc2 onto the extract-crates branch, following the /api/graph pick in the previous commit. /api/graph and /api/clusters were per-repository only: the repository resolves from the query or, failing that, the server's cwd. A desktop client running `serve` from the user's home directory has no repository at the cwd, so an "all repositories" request resolved to nothing and returned an empty graph. - 2716cf4 adds namespace-wide Leiden detection: one run over the combined, cross-repository graph rather than per repository. File nodes are qualified `repo:path`; symbol nodes keep their globally-unique FQNs and carry a `repository` field. - 9631bc2 exposes it as `?global=true[&namespace=]`, so a client can request a namespace-wide graph per request without restarting the server against a different namespace. Conflict resolved in src/lib.rs: the incoming re-export list still carried the memory types this branch removed, so ours was kept and only NAMESPACE_SCOPE_ID added. Verified against a real multi-repo index: the netatmo namespace returns 3812 nodes / 15946 edges / 180 communities where the per-repository path returned empty. Needed by Hoplon's namespace community-graph view.
Cherry-picks e49aa40 onto the extract-crates branch, completing the namespace graph work started in the previous two commits. `?global=` previously rejected `level=symbol` with "global supports level=file only: symbol communities are detected per repository", so the desktop client's Symbols graph could not load a namespace-wide view. This adds `build_namespace_symbol_graph` (one Leiden run over the combined call graph of every repository in the namespace) and wires it through the graph-view, clusters, and couplings paths. Symbol nodes keep their globally-unique FQNs and carry a `repository` field; file nodes stay qualified `repo:path`. Also carried from e49aa40: runtime LLM backend switching (`GET`/`POST /api/llm/target`, `PUT /api/llm/copilot/model`), so a native app can change backend without restarting the server. Conflicts resolved: - The memory subsystem this branch removed (memory_dream, dream, sessions, memory_controller, memory_tests, and the four memory MCP tools) was dropped rather than reintroduced, along with its endpoint-index entries. - Leiden moved to the `leiden` / `leiden_coupling` crates here, so the incoming re-exports through `cluster_detection` were replaced by direct crate imports. - `analyze_graph(.., id_prefix)` no longer exists: the crate's `analyze` returns generic results and `map_community` attaches codesearch's stable community id. The namespace path now uses that same two-step as the per-repository one. Verified against a real multi-repo index: `?level=symbol&global=true` returns 160 nodes / 1198 edges / 160 communities (auto-aggregated past the node limit) where it previously returned 400.
`/api/couplings` accepted a `namespace` query param and scoped its analysis to
it, but `/api/graph` and `/api/clusters` did not model the param at all — serde
silently dropped it and both fell back to the namespace `serve` was started in.
A client asking for one namespace therefore got a graph built from a different
one, while the couplings panel for the same view honoured the request. The
symptom is a community whose members belong to a repository that isn't in the
requested namespace, with couplings naming symbols from one that is.
Adds `namespace: Option<String>` to both endpoints' params and threads it into
`namespace_graph_view` / `create_namespace_clusters`, which already took an
`Option<&str>`.
Also gives `/api/symbol-clusters` the global form it was missing: it returned
400 ("symbol communities are detected per repository") even though
`create_namespace_symbol_communities` exists, so the symbol level had a
namespace-wide graph but no matching structured community list. The
management-server test that asserted that 400 now asserts the scoped 200.
Verified against a real multi-repo index — file level:
netatmo 919 nodes apidevice, apiuser, php-common
clustertest 95 nodes oem-api
search 2949 nodes apicamera, codesearch, matterjs
and symbol level scoping to the same repository sets. Previously every
namespace returned the same default-namespace graph.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/connector/api/container.rs (1)
591-614: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBlocking config load in
async fn new, and its error is silently swallowed.
CodesearchConfig::loaddoes a blockingstd::fs::read_to_string, called directly here instead of theload_asyncwrapper used elsewhere in this PR (e.g.llm.rs). ItsErris also discarded via.ok()— a corruptedconfig.jsonsilently falls back to the boot--llm-targetflag with no diagnostic, unlike sibling failure paths in this same file that log viawarn!.🔧 Proposed fix
- active_llm_target: std::sync::Mutex::new( - crate::connector::adapter::CodesearchConfig::load(&config.data_dir) - .ok() - .and_then(|c| c.active_llm_target()) - .unwrap_or(config.llm_target), - ), + active_llm_target: std::sync::Mutex::new( + match crate::connector::adapter::CodesearchConfig::load_async(&config.data_dir) + .await + { + Ok(cfg) => cfg.active_llm_target().unwrap_or(config.llm_target), + Err(e) => { + tracing::warn!( + "Failed to load persisted LLM target config, using boot default: {e}" + ); + config.llm_target + } + }, + ),Based on learnings and coding guidelines, "Use
async/awaitfor I/O and wrap blocking calls intokio::task::spawn_blocking" and "Do not silently swallow errors; log withtracing::warn!ortracing::error!before dropping an error when necessary."🤖 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 `@src/connector/api/container.rs` around lines 591 - 614, Update the async constructor around active_llm_target initialization to use CodesearchConfig::load_async with await instead of the blocking CodesearchConfig::load call. Preserve the persisted-target-over-flag fallback, but handle load errors explicitly by logging them with tracing::warn! before falling back to config.llm_target, matching the diagnostic behavior of sibling failure paths.Source: Coding guidelines
🧹 Nitpick comments (6)
src/connector/api/controller/couplings_controller.rs (1)
19-46: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame
global+repositorygap as the management API'scouplingshandler.This CLI command accepts both
repositoryand--globalwithout rejecting the conflict, unlikeclusters_controller's equivalent behavior in the REST API path. See the consolidated comment for the shared fix.🤖 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 `@src/connector/api/controller/couplings_controller.rs` around lines 19 - 46, Update the couplings method to reject requests where global is true and repository is provided, matching the validation behavior of clusters_controller. Perform this validation before choosing between namespace-wide and repository-specific detection, while preserving both valid execution paths.src/connector/adapter/management/handlers/couplings.rs (1)
19-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
global+repositoryconflict silently ignored instead of rejected.
clusters.rsandgraph_view.rs(GET /api/clusters,GET /api/graph) both reject?global=true&repository=...with a 400 (`repository` conflicts with `global`). This endpoint instead silently dropsrepositoryper the doc comment ("Ignored whenglobalis set"), so a caller who mistakenly supplies both gets a namespace-wide result with no indication theirrepositoryfilter was dropped — inconsistent with the sibling endpoints added in this same PR.🤖 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 `@src/connector/adapter/management/handlers/couplings.rs` around lines 19 - 39, The CouplingParams handling currently allows global=true with repository set and silently ignores the repository filter. Update the couplings endpoint validation to reject this combination with HTTP 400 using the same “repository conflicts with global” behavior as the clusters and graph_view handlers, while preserving valid repository-only and global-only requests.src/connector/adapter/management/handlers/graph_view.rs (1)
68-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates
ClusterParams::reject_global_with_repository().Same guard and error message as
clusters.rs, implemented inline here instead of via a shared helper. See consolidated comment.🤖 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 `@src/connector/adapter/management/handlers/graph_view.rs` around lines 68 - 74, Replace the inline global/repository conflict check in the GraphView handler with the shared ClusterParams::reject_global_with_repository() helper, preserving the existing validation behavior and error message while removing the duplicated guard.src/application/use_cases/cluster_detection.rs (1)
1-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFile continues to grow well past the 300-line guideline.
This module is now well over 1,100 lines even after the Leiden extraction, and this PR adds another ~150 lines of namespace-wide logic to it. Consider splitting the namespace-qualification helpers (
repo_labels,qualify_namespace_graph,NAMESPACE_NODE_SEPARATOR/REPO_LABEL_DISAMBIGUATION_LEN) andbuild_file_graph_viewinto a sibling module in a future pass.Based on coding guidelines: "Keep one logical concept per file; split files that grow beyond approximately 300 lines."
🤖 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 `@src/application/use_cases/cluster_detection.rs` around lines 1 - 32, Split the namespace-wide logic out of the oversized cluster detection module into a sibling module: move repo_labels, qualify_namespace_graph, NAMESPACE_NODE_SEPARATOR, REPO_LABEL_DISAMBIGUATION_LEN, and build_file_graph_view together, then update callers and visibility/imports so behavior remains unchanged.Source: Coding guidelines
src/connector/adapter/management/handlers/clusters.rs (1)
35-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood validation — consider extracting for reuse.
reject_global_with_repository()is a clean 400-guard for theglobal+repositoryconflict.graph_view.rs'sgraphhandler implements the identical check inline instead of reusing this (or a shared helper), andcouplings.rsskips the check entirely. Worth centralizing so the three endpoints can't drift.🤖 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 `@src/connector/adapter/management/handlers/clusters.rs` around lines 35 - 46, Centralize the global-plus-repository validation currently implemented by ClusterParams::reject_global_with_repository so it can be reused by the clusters, graph_view.rs graph handler, and couplings.rs endpoint. Replace the graph handler’s inline check and invoke the shared validation in couplings.rs, preserving the existing ApiError::bad_request response and allowing valid parameter combinations unchanged.src/connector/api/container.rs (1)
116-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFile exceeds the guideline's ~300-line split threshold.
container.rsspans roughly 900 lines and this PR continues to add fields/methods to it. Splitting the DI wiring (e.g. per-use-case factory modules) is a pre-existing, cross-cutting concern beyond this PR's scope, but flagging per guideline.As per coding guidelines, "Keep one logical concept per file; split files that grow beyond approximately 300 lines."
🤖 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 `@src/connector/api/container.rs` around lines 116 - 141, Split the oversized Container implementation into focused modules, keeping the Container struct and its dependency-injection wiring separated from per-use-case factory or construction logic. Preserve the existing public behavior and APIs while moving related methods into appropriately named modules and updating imports and module declarations.Source: Coding guidelines
🤖 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.
Inline comments:
In `@src/application/use_cases/coupling_detection.rs`:
- Around line 131-134: Update the comment describing the graph analysis and
stable ID derivation to replace the product-specific possessive codebase
reference with a generic phrase such as “the application’s,” while preserving
the rest of the explanation.
- Line 24: Move all direct leiden/leiden_coupling usage out of the application
use cases: update src/application/use_cases/coupling_detection.rs at lines 24-24
and src/application/use_cases/symbol_cluster_detection.rs at lines 228-228 to
depend on domain-facing ports or service boundaries instead. Place graph
construction, partitioning, and coupling analysis behind the appropriate domain
service or analysis repository abstraction, preserving the existing use-case
behavior while removing these infrastructure imports and calls from both files.
In `@src/application/use_cases/execution_features.rs`:
- Around line 91-121: Compute the traversal scope once per fixed repository_id
in compute_all_features/list_features, then pass that HashMap through
find_entry_points and build_feature. Remove their internal calls to
traversal_scope while preserving the existing scope behavior and reuse the same
map for every discovered entry point.
In `@src/application/use_cases/symbol_cluster_detection.rs`:
- Around line 520-527: Update the repository attribution logic around repo_of so
only the authoritative caller ownership is recorded. Remove the fallback
insertion that assigns the caller’s repository to callee, leaving unresolved
callees unlabeled unless their definition repository is resolved separately;
preserve the caller insertion behavior.
In `@src/connector/adapter/codesearch_config.rs`:
- Around line 27-33: Update the doc comment for the persisted llm_target field
to document OpenAI’s actual string form as "openai" without a hyphen, while
leaving the serialization and runtime behavior unchanged.
In `@src/connector/api/container.rs`:
- Around line 765-789: Update snippet_lookup_for_repository’s
metadata_repository().list() error branch to emit a tracing::warn! containing
the failure details before returning None, matching the existing
resolve_repository_id handling while preserving the current fallback behavior.
---
Outside diff comments:
In `@src/connector/api/container.rs`:
- Around line 591-614: Update the async constructor around active_llm_target
initialization to use CodesearchConfig::load_async with await instead of the
blocking CodesearchConfig::load call. Preserve the persisted-target-over-flag
fallback, but handle load errors explicitly by logging them with tracing::warn!
before falling back to config.llm_target, matching the diagnostic behavior of
sibling failure paths.
---
Nitpick comments:
In `@src/application/use_cases/cluster_detection.rs`:
- Around line 1-32: Split the namespace-wide logic out of the oversized cluster
detection module into a sibling module: move repo_labels,
qualify_namespace_graph, NAMESPACE_NODE_SEPARATOR,
REPO_LABEL_DISAMBIGUATION_LEN, and build_file_graph_view together, then update
callers and visibility/imports so behavior remains unchanged.
In `@src/connector/adapter/management/handlers/clusters.rs`:
- Around line 35-46: Centralize the global-plus-repository validation currently
implemented by ClusterParams::reject_global_with_repository so it can be reused
by the clusters, graph_view.rs graph handler, and couplings.rs endpoint. Replace
the graph handler’s inline check and invoke the shared validation in
couplings.rs, preserving the existing ApiError::bad_request response and
allowing valid parameter combinations unchanged.
In `@src/connector/adapter/management/handlers/couplings.rs`:
- Around line 19-39: The CouplingParams handling currently allows global=true
with repository set and silently ignores the repository filter. Update the
couplings endpoint validation to reject this combination with HTTP 400 using the
same “repository conflicts with global” behavior as the clusters and graph_view
handlers, while preserving valid repository-only and global-only requests.
In `@src/connector/adapter/management/handlers/graph_view.rs`:
- Around line 68-74: Replace the inline global/repository conflict check in the
GraphView handler with the shared ClusterParams::reject_global_with_repository()
helper, preserving the existing validation behavior and error message while
removing the duplicated guard.
In `@src/connector/api/container.rs`:
- Around line 116-141: Split the oversized Container implementation into focused
modules, keeping the Container struct and its dependency-injection wiring
separated from per-use-case factory or construction logic. Preserve the existing
public behavior and APIs while moving related methods into appropriately named
modules and updating imports and module declarations.
In `@src/connector/api/controller/couplings_controller.rs`:
- Around line 19-46: Update the couplings method to reject requests where global
is true and repository is provided, matching the validation behavior of
clusters_controller. Perform this validation before choosing between
namespace-wide and repository-specific detection, while preserving both valid
execution paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e99785d4-5b10-493e-887a-416d6ed53504
📒 Files selected for processing (40)
docs/features/architecture-analysis.mddocs/features/serve-and-management-api.mdsrc/application/interfaces/call_graph_repository.rssrc/application/use_cases/call_graph.rssrc/application/use_cases/cluster_detection.rssrc/application/use_cases/coupling_detection.rssrc/application/use_cases/delete_repository.rssrc/application/use_cases/execution_features.rssrc/application/use_cases/file_relationship.rssrc/application/use_cases/index_repository.rssrc/application/use_cases/symbol_cluster_detection.rssrc/application/use_cases/visualize_graph.rssrc/cli/mod.rssrc/connector/adapter/codesearch_config.rssrc/connector/adapter/duckdb_analysis_repository.rssrc/connector/adapter/duckdb_call_graph_repository.rssrc/connector/adapter/duckdb_vector_repository.rssrc/connector/adapter/management/copilot_login.rssrc/connector/adapter/management/handlers/clusters.rssrc/connector/adapter/management/handlers/couplings.rssrc/connector/adapter/management/handlers/graph_view.rssrc/connector/adapter/management/handlers/llm.rssrc/connector/adapter/management/server.rssrc/connector/adapter/management/streaming.rssrc/connector/api/container.rssrc/connector/api/controller/clusters_controller.rssrc/connector/api/controller/couplings_controller.rssrc/connector/api/controller/symbol_clusters_controller.rssrc/connector/api/controller/visualize_controller.rssrc/connector/api/router.rssrc/domain/models/cluster.rssrc/domain/models/execution_feature.rssrc/domain/models/graph_view.rssrc/lib.rstests/coupling_tests.rstests/duckdb_analysis_repository_tests.rstests/management_server_tests.rstests/namespace_cluster_tests.rstests/namespace_symbol_cluster_tests.rstests/repository_overview_tests.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/connector/adapter/management/server.rs
- tests/management_server_tests.rs
- docs/features/serve-and-management-api.md
- src/lib.rs
| }; | ||
| // Leiden itself moved out to the `leiden` / `leiden_coupling` crates on this | ||
| // branch, so only the graph builders still come from the file-level module. | ||
| use leiden_coupling::{analyze, CommunityCoupling as CrateCoupling, Coupler, CouplerKind}; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline src/application/interfaces --items all
rg -n --type rust 'use\s+(leiden|leiden_coupling)|\b(leiden|leiden_coupling)::' src/applicationRepository: ArtemisMucaj/codesearch
Length of output: 4823
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate files line counts =="
wc -l src/application/use_cases/*.rs src/application/interfaces/*.rs 2>/dev/null | sed -n '1,120p'
echo
echo "== coupling_detection relevant sections =="
cat -n src/application/use_cases/coupling_detection.rs | sed -n '1,220p'
echo
echo "== symbol_cluster_detection relevant sections =="
cat -n src/application/use_cases/symbol_cluster_detection.rs | sed -n '1,270p'
cat -n src/application/use_cases/symbol_cluster_detection.rs | sed -n '260,310p'
echo
echo "== cluster_detection relevant sections for direct leiden usage =="
cat -n src/application/use_cases/cluster_detection.rs | sed -n '1,70p'
cat -n src/application/use_cases/cluster_detection.rs | sed -n '480,520p'
cat -n src/application/use_cases/cluster_detection.rs | sed -n '1050,1090p'
echo
echo "== domain types / interfaces around graph community/coupling =="
rg -n --type rust 'type .*Community|struct .*Community|ClusterGraph|SymbolCommunityGraph|Coupling|CommunityCoupling|Coupler' src/domain src/application/use_cases src/application/interfacesRepository: ArtemisMucaj/codesearch
Length of output: 42728
Move Leiden graph operations out of application use cases.
src/application/use_cases/coupling_detection.rs:24 and src/application/use_cases/symbol_cluster_detection.rs:228 import and call leiden/leiden_coupling directly, which violates the application-layer boundary that should only depend on domain types and ports. Extract graph building, partitioning, and coupling analysis behind the domain-facing ports or service boundaries already shown for analysis repositories.
📍 Affects 2 files
src/application/use_cases/coupling_detection.rs#L24-L24(this comment)src/application/use_cases/symbol_cluster_detection.rs#L228-L228
🤖 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 `@src/application/use_cases/coupling_detection.rs` at line 24, Move all direct
leiden/leiden_coupling usage out of the application use cases: update
src/application/use_cases/coupling_detection.rs at lines 24-24 and
src/application/use_cases/symbol_cluster_detection.rs at lines 228-228 to depend
on domain-facing ports or service boundaries instead. Place graph construction,
partitioning, and coupling analysis behind the appropriate domain service or
analysis repository abstraction, preserving the existing use-case behavior while
removing these infrastructure imports and calls from both files.
Source: Coding guidelines
`llm_target` + `openai.active` answered "which backend", but the jobs here
differ in what they need: explaining a call flow wants a strong reasoner,
labelling a few hundred communities wants something cheap and fast. One shared
model forced the same trade-off on both.
Adds a `usages` map to `config.json`, keyed by `LlmUsage` — explain_code,
label_communities, summarize_overview, expand_queries. Each may name an
endpoint (including the reserved `copilot`), a model, or both; naming only a
model keeps the active backend and swaps the model. A usage with no entry
inherits the active backend, so the file stays empty until someone splits one
out and existing behaviour is unchanged.
GET /api/llm/usages every job, what answers it, and whether inherited
PUT /api/llm/usages/{id} bind one; empty body clears it
`build_chat_client_for(usage, ..)` reads config per call, so a change applies to
the next request without restarting serve. The overview controller previously
used one client for both community naming and the closing executive summary;
those are now separate usages, since one is a short per-cluster call and the
other reasons over the whole report.
Query expansion is the exception: its client is pinned when serve boots, so its
binding needs a restart. The API reports that as `requires_restart` rather than
letting the setting look broken.
Also adds `OpenAiChatClient::from_config_with_model`, mirroring the Copilot
client's existing `from_data_dir_with_model`, so a usage can override the model
on a resolved endpoint.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/connector/api/controller/overview_controller.rs (1)
78-108: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not gate the summary on label-client initialization.
The
SummarizeOverviewclient is built only insideOk(chat). A failedlabel_communitiesendpoint therefore skips a separately configured, working summary backend. Run the summary branch after the naming-client match.🤖 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 `@src/connector/api/controller/overview_controller.rs` around lines 78 - 108, Move the OverviewSection::Summary generation branch out of the Ok(chat) arm of the build_chat_client_for(LlmUsage::LabelCommunities, ...) match. Keep community and module naming inside the naming-client success path, then independently build SummarizeOverview and generate report.summary when enabled, including its existing warning behavior on failure.
🤖 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.
Inline comments:
In `@src/connector/api/controller/mod.rs`:
- Around line 53-65: Update the override handling around
OpenAiChatClient::from_config_with_model so model-only bindings
(binding.endpoint is None and binding.model is Some) remain on the active
backend selected by llm. Route Copilot through
CopilotChatClient::from_data_dir_with_model, handle Anthropic model overrides
explicitly by implementing their supported path or returning a clear unsupported
error, and only construct the OpenAI client for OpenAI-compatible backends or
endpoint overrides.
- Line 42: Update the configuration loading in the controller flow around
CodesearchConfig::load to propagate failures instead of calling
unwrap_or_default. Return a contextual error from the enclosing function so
callers can surface it, while preserving successful configuration loading and
downstream override behavior.
---
Outside diff comments:
In `@src/connector/api/controller/overview_controller.rs`:
- Around line 78-108: Move the OverviewSection::Summary generation branch out of
the Ok(chat) arm of the build_chat_client_for(LlmUsage::LabelCommunities, ...)
match. Keep community and module naming inside the naming-client success path,
then independently build SummarizeOverview and generate report.summary when
enabled, including its existing warning behavior on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 923ea851-efff-43b5-a85d-980659c276a9
📒 Files selected for processing (10)
src/connector/adapter/codesearch_config.rssrc/connector/adapter/management/handlers/llm.rssrc/connector/adapter/management/server.rssrc/connector/adapter/openai_chat_client.rssrc/connector/api/container.rssrc/connector/api/controller/clusters_controller.rssrc/connector/api/controller/explain_controller.rssrc/connector/api/controller/mod.rssrc/connector/api/controller/overview_controller.rssrc/connector/api/controller/symbol_clusters_controller.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- src/connector/api/controller/clusters_controller.rs
- src/connector/adapter/management/handlers/llm.rs
- src/connector/adapter/management/server.rs
- src/connector/api/controller/symbol_clusters_controller.rs
- src/connector/api/container.rs
- src/connector/adapter/openai_chat_client.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.claude/skills/codesearch-cli/SKILL.md:
- Around line 80-82: Update the nearby “Weak” example in the phase guidance to
reference Phase 3, directing users with an exact symbol name to context/impact
instead of Phase 4. Keep the example’s remaining guidance unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dd5db29b-6e74-4bd3-9a8e-78717c41121c
📒 Files selected for processing (2)
.claude/skills/codesearch-cli/SKILL.md.claude/skills/codesearch-mcp/SKILL.md
added 2 commits
August 3, 2026 12:18
…ct-crates-drop-memory # Conflicts: # src/application/use_cases/coupling_detection.rs # src/application/use_cases/memory_dream.rs # src/application/use_cases/symbol_cluster_detection.rs # src/connector/adapter/copilot_chat_client.rs # src/connector/adapter/management/dream.rs # src/connector/adapter/management/handlers/clusters.rs # src/connector/adapter/management/handlers/graph_view.rs # src/connector/adapter/management/handlers/llm.rs # src/connector/adapter/management/handlers/sessions.rs # src/connector/adapter/management/server.rs # src/connector/adapter/management/streaming.rs # src/connector/adapter/mcp/server.rs # src/connector/adapter/openai_chat_client.rs # src/connector/api/container.rs # src/connector/api/controller/memory_controller.rs # src/lib.rs # tests/management_server_tests.rs # tests/memory_tests.rs
Domain layer no longer depends on the LLM crates: the `From<OpenAiError>` / `From<CopilotError>` impls move out of `src/domain/error.rs` into `connector/adapter/llm_error.rs` as `map_openai_err` / `map_copilot_err`, restoring the inward-pointing dependency rule. Both mappers walk the error source chain, which `to_string()` alone dropped. Correctness and routing: - `build_chat_client_for` kept model-only overrides on the OpenAI backend even when Copilot or Anthropic was active, so a usage that named only a model silently answered from a different provider. Overrides without an endpoint now resolve against the active backend, and unsupported Anthropic model overrides are rejected rather than rerouted. - The same function swallowed config-load failures via `unwrap_or_default()`, degrading a malformed config into "no overrides"; it now propagates. - `OpenAiChatClient::list_models` rebuilt its endpoint from `base_url` alone, dropping the API key and timeout, so discovery 401'd against key-protected servers while chat worked. The configured `Endpoint` is retained and reused. - Symbol communities attributed a callee to the caller's repository as a first-seen guess; a cross-repository leaf callee never got corrected and was published with the wrong owner. Only authoritative caller ownership is recorded now, leaving unresolved callees unlabeled. - `CopilotChatClient::new` accepted an empty model id and sent `""` upstream; it now fails with an actionable message. Robustness and performance: - Both embedding adapters `.expect()`-ed on a fallible client build, aborting the process on a bad base URL; `new()` is now fallible. - `compute_all_features` resolved the traversal scope once per entry point, re-running `repositories.list()` N+1 times for an identical result; it is resolved once and lent to each step. - `snippet_lookup_for_repository` dropped a repository-listing error silently. Tests and docs: - The channel-filter test asserted only status and shape, so a handler that ignored the filter still passed. It now indexes the two messaging fixtures as separate repositories and asserts the filtered report excludes the repository it does not name. - Correct the MCP tool count (16, not 20), drop stale memory references from the architecture overview and serve docs, fix a stale phase reference in the codesearch-cli skill, and correct the persisted `llm_target` string form (`"openai"`, not `"open-ai"`). - Ignore `*.scip` indexes, which indexing and tests emit as build artifacts.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/connector/adapter/management/handlers/llm.rs (4)
440-442: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not replace a corrupted configuration with defaults.
CodesearchConfig::loadreturns an error for a present malformed or unreadable file.unwrap_or_defaultdiscards that error. Aset_usagerequest can then save a default configuration and remove existing endpoints,llm_target, or Copilot credentials.Return the load error before mutating or saving the configuration.
Based on coding guidelines, errors must not be silently swallowed.
🤖 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 `@src/connector/adapter/management/handlers/llm.rs` around lines 440 - 442, Update the configuration-loading flow around CodesearchConfig::load to propagate its error instead of using unwrap_or_default. Ensure set_usage returns the load failure before mutating or saving cfg, while preserving default initialization only when the configuration is genuinely absent according to the existing load behavior.Source: Coding guidelines
459-468: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject bindings that the resolver cannot serve.
A model-only binding is accepted while
LlmTarget::Anthropicis active.src/connector/api/controller/mod.rsLines 60-95 explicitly rejects that combination. The endpoint can therefore return success for a configuration that makes this usage fail.Reject model-only bindings for Anthropic, and reject blank or whitespace-only model identifiers before saving.
🤖 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 `@src/connector/adapter/management/handlers/llm.rs` around lines 459 - 468, Validate the binding before mutating cfg.usages in the handler around UsageBinding: reject model-only bindings when the active LlmTarget is Anthropic, matching the validation in the LlmTarget resolver, and reject model identifiers that are empty or contain only whitespace. Return the appropriate validation error instead of saving either invalid binding.
411-420: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse fallible, non-blocking config loading in
list_usages.
CodesearchConfig::loadruns blocking file I/O, andunwrap_or_default()hides a malformed config by returning the default instead of reporting the error. Load the file off the async runtime withCodesearchConfig::load_async(state.container.data_dir()).await?and let read/parse errors propagate.🤖 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 `@src/connector/adapter/management/handlers/llm.rs` around lines 411 - 420, Update list_usages to call CodesearchConfig::load_async with the container data directory, await the result, and propagate read or parse errors with ?. Remove the blocking CodesearchConfig::load and unwrap_or_default fallback while preserving the existing usage_json mapping and response structure.Source: Coding guidelines
422-429: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep the clear operation explicit for malformed requests.
In JSON, Serde ignores unknown fields unless
deny_unknown_fieldsis set. A misspelledendpoint/modelfield can deserialize as an empty body and delete the usage binding. Use#[serde(deny_unknown_fields)]onSetUsageBody, or add a separate clear endpoint so accidental typos do not remove an override.🤖 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 `@src/connector/adapter/management/handlers/llm.rs` around lines 422 - 429, Update the SetUsageBody deserialization to reject unknown JSON fields by applying Serde’s deny_unknown_fields behavior, ensuring misspelled endpoint or model fields fail validation instead of being interpreted as an intentional clear operation.
🧹 Nitpick comments (1)
src/connector/adapter/management/handlers/llm.rs (1)
440-442: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftUse one non-blocking config path for per-usage routing.
build_chat_client_forreadsconfig.jsonsynchronously, andOpenAiChatClient::from_config_with_modelalso reads synchronously when it receives the resolved data dir. Make both fallible; run the blocking reads throughspawn_blockingor make the helpers async so no async handler stalls the executor.🤖 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 `@src/connector/adapter/management/handlers/llm.rs` around lines 440 - 442, Make the config-loading paths in build_chat_client_for and OpenAiChatClient::from_config_with_model fallible and non-blocking by moving synchronous config.json reads into spawn_blocking or converting the helpers to async, while preserving per-usage routing behavior. Apply the same change at src/connector/adapter/management/handlers/llm.rs:440-442 and src/connector/api/controller/symbol_clusters_controller.rs:53-54; update callers to await and propagate loading errors.Source: Coding guidelines
🤖 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.
Inline comments:
In `@src/connector/adapter/management/handlers/llm.rs`:
- Around line 381-391: Update the model-selection logic in the handler’s `let
model` expression so it follows the effective client route: use
`cfg.copilot.model` only when `name` resolves to `COPILOT_ENDPOINT`, not merely
when `active` is `LlmTarget::Copilot`; ensure bindings to OpenAI endpoints
continue resolving their configured OpenAI model. Also correct the fallback near
the surrounding active-model logic so `LlmTarget::Anthropic` reports the
Anthropic configuration rather than `cfg.openai.active`, matching the routing
behavior in the controller.
---
Outside diff comments:
In `@src/connector/adapter/management/handlers/llm.rs`:
- Around line 440-442: Update the configuration-loading flow around
CodesearchConfig::load to propagate its error instead of using
unwrap_or_default. Ensure set_usage returns the load failure before mutating or
saving cfg, while preserving default initialization only when the configuration
is genuinely absent according to the existing load behavior.
- Around line 459-468: Validate the binding before mutating cfg.usages in the
handler around UsageBinding: reject model-only bindings when the active
LlmTarget is Anthropic, matching the validation in the LlmTarget resolver, and
reject model identifiers that are empty or contain only whitespace. Return the
appropriate validation error instead of saving either invalid binding.
- Around line 411-420: Update list_usages to call CodesearchConfig::load_async
with the container data directory, await the result, and propagate read or parse
errors with ?. Remove the blocking CodesearchConfig::load and unwrap_or_default
fallback while preserving the existing usage_json mapping and response
structure.
- Around line 422-429: Update the SetUsageBody deserialization to reject unknown
JSON fields by applying Serde’s deny_unknown_fields behavior, ensuring
misspelled endpoint or model fields fail validation instead of being interpreted
as an intentional clear operation.
---
Nitpick comments:
In `@src/connector/adapter/management/handlers/llm.rs`:
- Around line 440-442: Make the config-loading paths in build_chat_client_for
and OpenAiChatClient::from_config_with_model fallible and non-blocking by moving
synchronous config.json reads into spawn_blocking or converting the helpers to
async, while preserving per-usage routing behavior. Apply the same change at
src/connector/adapter/management/handlers/llm.rs:440-442 and
src/connector/api/controller/symbol_clusters_controller.rs:53-54; update callers
to await and propagate loading errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 040a0d76-c2a2-46c1-9ee9-713b71e34eee
📒 Files selected for processing (24)
.claude/skills/codesearch-cli/SKILL.md.gitignoreCargo.tomlREADME.mddocs/architecture/overview.mddocs/features/serve-and-management-api.mdsrc/application/use_cases/execution_features.rssrc/application/use_cases/symbol_cluster_detection.rssrc/connector/adapter/codesearch_config.rssrc/connector/adapter/copilot_chat_client.rssrc/connector/adapter/llm_error.rssrc/connector/adapter/lm_studio_embedding.rssrc/connector/adapter/management/handlers/llm.rssrc/connector/adapter/mod.rssrc/connector/adapter/openai_chat_client.rssrc/connector/adapter/openai_embedding.rssrc/connector/api/container.rssrc/connector/api/controller/clusters_controller.rssrc/connector/api/controller/explain_controller.rssrc/connector/api/controller/mod.rssrc/connector/api/controller/overview_controller.rssrc/connector/api/controller/symbol_clusters_controller.rssrc/lib.rstests/management_server_tests.rs
🚧 Files skipped from review as they are similar to previous changes (15)
- Cargo.toml
- src/connector/api/controller/explain_controller.rs
- src/application/use_cases/execution_features.rs
- docs/features/serve-and-management-api.md
- src/connector/api/controller/overview_controller.rs
- README.md
- src/connector/api/controller/mod.rs
- src/connector/api/controller/clusters_controller.rs
- .claude/skills/codesearch-cli/SKILL.md
- docs/architecture/overview.md
- src/connector/adapter/mod.rs
- src/connector/api/container.rs
- src/lib.rs
- src/connector/adapter/copilot_chat_client.rs
- src/application/use_cases/symbol_cluster_detection.rs
added 2 commits
August 3, 2026 14:10
The `../leiden-rs`, `../openai-rs` and `../gh-copilot-rs` path dependencies
only resolved when the author happened to have sibling checkouts, so a clean
checkout and CI both failed before compiling any project code:
error: failed to get `gh-copilot-rs` as a dependency
unable to update /home/runner/work/codesearch/gh-copilot-rs
Point each at its public repository, pinned to the revision this branch was
built and tested against, so builds are reproducible and CI resolves them
without any local layout assumption.
The strengthened filter test indexed the JS notification fixture, which routes
through `scip-typescript`. That binary is not on CI's PATH, so the test failed
there while passing locally where it happens to be installed:
failed to index notification-service: SCIP indexer failed:
'scip-typescript' was not found on PATH.
Use a Python consumer fixture instead. Python parsing is pure tree-sitter, so
the test exercises the same producer/consumer channel graph with no external
indexer — as the testing strategy requires. Verified with the node bin
directory stripped from PATH, and confirmed still discriminating: stubbing the
handler's repository filter fails the assertion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Splits three subsystems out of codesearch into their own crates, leaving this
repo focused on code intelligence, and adds the one serve endpoint a native
client needs to draw the community graph.
What moves out
The memory subsystem and the session-feeding ring are removed here; the
/api/memory*and/api/sessions*routes go with them. Memory is now aseparate service with its own store, config and HTTP API.
leiden-rs. Drops thepetgraphdependency.
openai-rsandgh-copilot-rs. Chat, embeddings,reranking and Copilot login all go through the shared crates now.
Net: 76 files, ~17k lines removed.
What comes in
GET /api/graph(cherry-picked from #202)./api/clustersand/api/symbol-clustersreturn community membership only — no edge adjacency — soa client can't draw the community graph the
visualizeCLI renders. Thisreturns the full
GraphView(nodes + edges + communities) for one repository atthe file or symbol level, reusing the same
graph_view()builders the visualizepath drives, and auto-aggregating large graphs into the community meta-graph.
It also carries #202's
/api/channelsfix: therepositoryfilter wasOption<Vec<String>>, which axum's defaultQuerycannot deserialize from anyquery form, so the filter silently never bound and the endpoint always fell back
to the cwd namespace.
The cherry-pick applies cleanly over the leiden-rs migration.
Why now
The Hoplon desktop app supervises
codesearch and memory-rs side by side. Keeping memory in both would mean two
services writing the same concern;
/api/graphis what its namespacecommunity-graph view renders.
Verification
cargo test— 384 passed, 2 ignored, 18 suites. Management server suite coversthe new endpoint.
Summary by CodeRabbit