Skip to content

Implement graph edges collection support with admin and ingest updates - #2

Open
voarsh2 wants to merge 67 commits into
mit-base-salvagefrom
mit-base-salvage-symbol
Open

voarsh2 wants to merge 67 commits into
mit-base-salvagefrom
mit-base-salvage-symbol

Conversation

@voarsh2

@voarsh2 voarsh2 commented Feb 9, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Graph-edge accelerator with companion "_graph" collections, optional backfill, CLI cache-clear flag, delta plan/apply endpoints, path-scope utilities, and bundled VS Code bridge mode.
  • Bug Fixes

    • Companion graph copy/delete made best-effort/non-fatal; more resilient handling for vector-less collections and index operations.
  • Improvements

    • Admin UI surfaces graph state in redirects; clearer indexing/watch journaling, queue de-duplication, upload reporting, and debug-aware search output.
  • Tests

    • Broad new/updated tests covering ingest, graph/backfill, delta plan/apply, path-scope, queue, consistency, upload, and admin flows.

…arity

feat(graph-edges): add qdrant _graph backfill + symbol_graph accel; mirror admin lifecycle
feat: materialize _graph edges and use for symbol_graph; ensure clone/delete handles companion collections
graph: add _graph edge collection + backfill; prune/watcher cleanup; admin copy/delete parity
feat(admin+graph): copy/delete companion _graph collections; add UI status + tests
Adds a command-line option to clear indexing caches before running the ingest process.

This ensures a clean indexing run by removing any stale file hash or symbol caches.  This is useful for scenarios where the underlying code has changed significantly, invalidating the existing cache.

Also, ensures `CTXCE_FORCE_COLLECTION_NAME` disables multi-repo enumeration in `ingest_code`, forcing the use of the specified collection name, and clarifies its purpose in `indexing_admin.py`.

Broken by commit 2e6317d
@coderabbitai

coderabbitai Bot commented Feb 9, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: f814b88c-654d-4d07-9f65-05a8101a31d0

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a materialized graph-edge subsystem with per-collection <base>_graph collections, best-effort sync/backfill, and integration into indexing, delete, and copy flows; introduces path-scope utilities, index journaling, delta plan/apply flows, upload client/service enhancements, maintenance tooling, VSCode bridge updates, and CI workflow.

Changes

Cohort / File(s) Summary
Graph edges core & integration
scripts/ingest/graph_edges.py, scripts/ingest/pipeline.py, scripts/ingest/qdrant.py, scripts/prune.py, scripts/mcp_impl/symbol_graph.py
New graph_edges module materializes <base>_graph collections, provides deterministic payload-only upserts/deletes, and a backfill tick. Pipeline and qdrant integration perform best-effort syncs; symbol_graph queries the graph first and falls back to legacy arrays. prune/delete API accepts optional repo.
Collection admin & indexing admin + UI/tests
scripts/collection_admin.py, scripts/indexing_admin.py, scripts/upload_service.py, templates/admin/acl.html, tests/test_admin_collection_delete.py, tests/test_staging_lifecycle.py
Admin delete/copy attempt best-effort companion <name>_graph delete/copy and surface graph flags (qdrant_graph_deleted/graph_copied) in redirects/UI. Admin-spawned ingest now sets CTXCE_FORCE_COLLECTION_NAME in env; tests updated to match.
Ingest CLI, façade exports & CLI tests
scripts/ingest/cli.py, scripts/ingest_code.py, tests/test_ingest_cli.py
Adds --clear-indexing-caches CLI flag (repo/workspace-scoped) and conditional cache clearing. Re-exports is_text_like_language. Adds optional graph-edge façade exports with ImportError fallbacks. Adjusts multi-repo logic when force-collection env is set.
Indexing pipeline, pseudo & processor integration
scripts/ingest/pipeline.py, scripts/ingest/pseudo.py, scripts/watch_index_core/processor.py, scripts/watch_index_core/pseudo.py
Exposes is_text_like_language, threads preloaded_text/file_hash/language through indexing, centralizes pseudo cache lookup, propagates pseudo/tags into symbol metadata, and performs best-effort graph-edge sync calls at key upsert/delete/exclude points.
Watch/index core — maintenance, queue, handler, paths
scripts/watch_index_core/consistency.py, scripts/watch_index_core/queue.py, scripts/watch_index_core/handler.py, scripts/watch_index_core/paths.py, scripts/watch_index_core/config.py, scripts/watch_index_core/utils.py, scripts/watch_index.py
Adds consistency audit and empty-dir sweep maintenance, ChangeQueue fingerprinting/force support and RECENT_FINGERPRINT_TTL_SECS, internal metadata path helpers, refined move/delete handling across internal boundaries, and periodic maintenance hooks.
Workspace state & index journal
scripts/workspace_state.py, scripts/watch_index_core/processor.py
Introduces index-journal APIs (upsert/list/update), MaintenanceInfo types, per-repo symbol cache APIs (get/set symbols/pseudo), enhanced collection mappings, and processor integration for journal-driven work.
Delta planning, bundle processing & upload clients/service
scripts/upload_delta_bundle.py, scripts/upload_service.py, scripts/remote_upload_client.py, scripts/standalone_upload_client.py
Adds plan_delta_upload and apply_delta_operations, richer bundle parsing and apply-only flows, client-side planning/cache helpers, async upload polling, server plan/apply endpoints and status tracking, and extensive upload/watch orchestration improvements.
Qdrant helpers & payload-index memoization
scripts/ingest/qdrant.py, scripts/ingest/graph_edges.py
Adds ENSURED_PAYLOAD_INDEX_COLLECTIONS memoization, allows vector_name: None in ensure_collection_and_indexes_once, validates payload-index presence, and wires graph-edge collection creation/usage.
Path-scope & hybrid/rerank changes
scripts/path_scope.py, scripts/hybrid/expand.py, scripts/hybrid_search.py, scripts/rerank_tools/local.py
New path-scope utilities (normalize_under, metadata_matches_under, path_matches_under); moves under-filtering to client-side recursive metadata checks and applies metadata-based scoping across hybrid, dense, and rerank flows.
Watcher queue/process tests & utilities
tests/... (index_journal, watch_queue, path_scope, rerank, upload_service, smart-reindex, many more)
Large set of new and updated tests covering index journal semantics, ChangeQueue fingerprint/force behavior, path_scope normalization, rerank under-scope behavior, delta plan/apply scenarios, smart reindex/pseudo caching, and asyncio.run conversions.
Bridge & VSCode extension
ctx-mcp-bridge/src/mcpServer.js, vscode-extension/...
Bridge: resource listing, composite cursor helpers, timeouts, session/default sync, and hardened list operations. VSCode extension: bundled vs external bridge mode, bundled-bridge discovery, extensionRoot wiring, and bundled-python libs discovery with inflight dep-check guards.
Misc tooling & CI
scripts/ingest_history.py, scripts/codex_phase3_probe.py, .github/workflows/claude.yaml, vscode-extension/build/build.sh
ingest_history: logging and batched upserts with improved failure accounting; tiny codex probe script; GitHub workflow to run Claude Code; build script bundling the MCP bridge into the VSCode extension.

Sequence Diagrams

sequenceDiagram
    participant Client
    participant Pipeline
    participant GraphModule as graph_edges
    participant Qdrant

    Client->>Pipeline: upsert file (points + calls/imports)
    activate Pipeline
    Pipeline->>GraphModule: ensure_graph_collection(base)
    activate GraphModule
    GraphModule->>Qdrant: create/verify <base>_graph collection
    Qdrant-->>GraphModule: collection ready
    deactivate GraphModule

    Pipeline->>GraphModule: upsert_file_edges(caller_path, calls, imports, repo)
    activate GraphModule
    GraphModule->>Qdrant: upsert payload-only edge docs (deterministic IDs)
    Qdrant-->>GraphModule: upsert ack
    deactivate GraphModule

    Pipeline->>GraphModule: delete_edges_by_path(caller_path, repo)
    activate GraphModule
    GraphModule->>Qdrant: delete matching payload-only edge docs
    Qdrant-->>GraphModule: deletion result
    deactivate GraphModule

    Pipeline-->>Client: indexing complete
    deactivate Pipeline
Loading
sequenceDiagram
    participant UserQuery
    participant SymbolGraph
    participant GraphModule as graph_edges
    participant Qdrant

    UserQuery->>SymbolGraph: query_callers(symbol, under?, language?)
    activate SymbolGraph
    SymbolGraph->>GraphModule: query <base>_graph filter edge_type=calls (+filters)
    activate GraphModule
    GraphModule->>Qdrant: scroll query for edge docs
    Qdrant-->>GraphModule: edge documents
    GraphModule->>SymbolGraph: hydrated caller results
    deactivate GraphModule

    alt graph results exist
        SymbolGraph-->>UserQuery: return hydrated results
    else
        SymbolGraph->>Qdrant: fallback query on callers[] array field
        Qdrant-->>SymbolGraph: legacy results
        SymbolGraph-->>UserQuery: return legacy results
    end
    deactivate SymbolGraph
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mit-base-salvage-symbol

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

I hop through nodes and payload lanes,
Stitching callers, imports, tiny chains,
Backfills tick while edges quietly bloom,
Journals whisper orders, queues clear the room,
A rabbit tends your graph and keeps code groomed. 🐇✨

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@scripts/mcp_impl/symbol_graph.py`:
- Around line 342-352: Inner functions _scroll and _scroll_main close over loop
variables (flt and p) by reference which can cause latent bugs; change their
definitions to bind the current loop value explicitly (e.g., add flt=None /
p=None as default parameters and use that inside, or accept the value as a
parameter) so each coroutine captures the intended value when created; update
the calls that invoke _scroll() and _scroll_main() to pass no args (if using
defaults) or the bound value accordingly to ensure each task uses the correct
flt and p.
🧹 Nitpick comments (15)
scripts/remote_upload_client.py (1)

523-557: _excluded_dirnames() rebuilds the set on every _is_ignored_path call — consider caching.

Since _is_ignored_path is called for every path in detect_file_changes, get_all_code_files, and the event handler, _excluded_dirnames() reconstructs the set each time. The env vars it depends on won't change mid-process.

Also, parts = set(rel.parts) on Line 544 includes the filename itself, so a code file literally named build, dist, out, etc. (no extension) would be incorrectly ignored. This is unlikely for real code files but worth noting.

♻️ Cache the excluded set (optional)
+    `@functools.cached_property`
+    def _excluded_dirs(self) -> frozenset:
+        excluded = {
+            "node_modules", "vendor", "dist", "build", "target", "out",
+            ".git", ".hg", ".svn", ".vscode", ".idea", ".venv", "venv",
+            "__pycache__", ".pytest_cache", ".mypy_cache", ".cache",
+            ".context-engine", ".context-engine-uploader", ".codebase",
+        }
+        dev_remote = os.environ.get("DEV_REMOTE_MODE") == "1" or os.environ.get("REMOTE_UPLOAD_MODE") == "development"
+        if dev_remote:
+            excluded.add("dev-workspace")
+        return frozenset(excluded)

And to fix the filename false-positive, check only directory parts:

-        parts = set(rel.parts)
-        if parts & self._excluded_dirnames():
+        dir_parts = set(rel.parts[:-1]) if len(rel.parts) > 1 else set()
+        if dir_parts & self._excluded_dirnames():
scripts/standalone_upload_client.py (1)

733-763: Identical logic to remote_upload_client.py — keep-in-sync comment is appreciated but fragile.

The "Keep in sync with get_all_code_files exclusions" comment helps, but these two files now share ~30 lines of identical exclusion logic. Same note as the other file applies: _excluded_dirnames() is rebuilt per call, and parts = set(rel.parts) includes the filename.

Since the standalone client is intentionally dependency-free, the duplication is understandable. Consider at minimum adding a test that asserts the excluded dir sets match across both clients to catch drift.

scripts/collection_admin.py (1)

493-505: Log the graph-copy failure instead of silently swallowing it.

The recursive copy_collection_qdrant call is properly guarded against infinite recursion (the _graph suffix check). However, silently passing on exceptions makes debugging difficult when graph copies fail unexpectedly.

♻️ Add minimal logging
     if not src.endswith("_graph") and not dest.endswith("_graph"):
         try:
             copy_collection_qdrant(
                 source=f"{src}_graph",
                 target=f"{dest}_graph",
                 qdrant_url=base_url,
                 overwrite=overwrite,
             )
-        except Exception:
-            pass
+        except Exception as exc:
+            import logging as _logging
+            _logging.getLogger(__name__).debug(
+                "Best-effort graph collection copy %s_graph -> %s_graph failed: %s",
+                src, dest, exc,
+            )
scripts/ingest/graph_edges.py (2)

109-144: Broad except on vector-less creation attempt may mask transient errors.

Line 117 catches any exception from the vector-less create_collection call and falls back to the named-vector variant. If the failure is a transient network error rather than "unsupported config," the fallback will likely also fail — so the net effect is acceptable. Just noting this for awareness.

♻️ Optional: log the fallback reason at debug level
         try:
             client.create_collection(
                 collection_name=graph_coll,
                 vectors_config={},
             )
             _GRAPH_VECTOR_MODE[graph_coll] = "none"
-        except Exception:
+        except Exception as vec_exc:
+            logger.debug("Vector-less creation failed for %s, trying named vector: %s", graph_coll, vec_exc)
             client.create_collection(

147-149: Edge ID uses : as a separator — consider a more robust delimiter.

If any component (repo, path, symbol) contains :, different edge tuples could produce the same hash input. E.g., "calls:a:b:c:d" is ambiguous. A null byte (\x00) would be safer as a delimiter since it doesn't appear in file paths or symbol names.

This is low-risk in practice since paths and symbols rarely contain colons.

♻️ Use null byte separator
 def _edge_id(edge_type: str, repo: str, caller_path: str, callee_symbol: str) -> str:
-    key = f"{edge_type}:{repo}:{caller_path}:{callee_symbol}"
+    key = f"{edge_type}\x00{repo}\x00{caller_path}\x00{callee_symbol}"
     return hashlib.sha256(key.encode("utf-8", errors="ignore")).hexdigest()[:32]
scripts/mcp_impl/symbol_graph.py (2)

27-28: GRAPH_COLLECTION_SUFFIX is duplicated from scripts/ingest/graph_edges.py.

Both files define GRAPH_COLLECTION_SUFFIX = "_graph". This is a minor DRY concern; consider importing from graph_edges or extracting to a shared constants module if the two modules will evolve together.


381-432: N+1 hydration queries — acceptable given the limit cap.

Each caller path triggers a separate scroll call against the main collection (lines 400-408). With the default limit of 20, this means up to 20 sequential queries. For the current use case this is fine, but if limit grows significantly, consider batching hydration using a single scroll with a MatchAny filter on metadata.path.

scripts/upload_service.py (1)

1241-1273: Import pooled_qdrant_client and use it instead of creating a standalone QdrantClient.

The rest of the codebase uses pooled_qdrant_client for Qdrant interactions. Here, a new QdrantClient is instantiated just to check if the _graph collection exists. Using the pooled client would be more consistent and avoid creating an extra connection. This requires importing pooled_qdrant_client from scripts.qdrant_client_manager.

♻️ Suggested refactor using pooled client

At the top of the file with other imports, add:

try:
    from scripts.qdrant_client_manager import pooled_qdrant_client
except Exception:
    pooled_qdrant_client = None

Then replace the collection check logic:

     graph_copied: Optional[str] = None
     try:
         if not name.endswith("_graph") and not str(new_name).endswith("_graph"):
-            from qdrant_client import QdrantClient  # type: ignore
-
-            cli = QdrantClient(
-                url=QDRANT_URL,
-                api_key=os.environ.get("QDRANT_API_KEY"),
-                timeout=float(os.environ.get("QDRANT_TIMEOUT", "5") or 5),
-            )
-            try:
-                cli.get_collection(collection_name=f"{new_name}_graph")
-                graph_copied = "1"
-            except Exception:
-                graph_copied = "0"
-            finally:
-                try:
-                    cli.close()
-                except Exception:
-                    pass
+            if pooled_qdrant_client is not None:
+                with pooled_qdrant_client(url=QDRANT_URL, api_key=os.environ.get("QDRANT_API_KEY")) as cli:
+                    try:
+                        cli.get_collection(collection_name=f"{new_name}_graph")
+                        graph_copied = "1"
+                    except Exception:
+                        graph_copied = "0"
     except Exception:
         graph_copied = None
scripts/ingest/cli.py (1)

195-209: Remove duplication — reuse scripts/collection_health.clear_indexing_caches instead.

scripts/collection_health.py already has a clear_indexing_caches(workspace_path, repo_name) function that clears both file-hash and symbol caches. It uses public clear_repo_cache/clear_cache APIs instead of relying on private helpers (_ws._get_repo_state_dir, _ws._get_cache_path), which are fragile if workspace_state internals change. The existing implementation also handles errors more gracefully.

Since the return value is discarded at all call sites, wrap the existing function in a thin adapter:

♻️ Reuse existing helper
+from scripts.collection_health import clear_indexing_caches as _clear_indexing_caches_impl
+
 ...
 
-    def _clear_indexing_caches(workspace_root: Path, repo_name: str | None) -> None:
-        try:
-            _ws.clear_symbol_cache(workspace_path=str(workspace_root), repo_name=repo_name)
-        except Exception:
-            pass
-        try:
-            if _ws.is_multi_repo_mode() and repo_name:
-                state_dir = _ws._get_repo_state_dir(repo_name)
-                cache_path = state_dir / _ws.CACHE_FILENAME
-            else:
-                cache_path = _ws._get_cache_path(workspace_root)
-            if cache_path.exists():
-                cache_path.unlink()
-        except Exception:
-            pass
+    def _clear_indexing_caches(workspace_root: Path, repo_name: str | None) -> None:
+        try:
+            _clear_indexing_caches_impl(str(workspace_root), repo_name=repo_name)
+        except Exception:
+            pass
scripts/ingest/pipeline.py (2)

656-689: Silent except Exception: pass swallows errors without any diagnostic trace.

While this block is intentionally best-effort, silently swallowing all exceptions (including unexpected ones like TypeError or AttributeError from API misuse) makes production debugging very difficult. A debug-level log would preserve the "safe to skip" intent while aiding troubleshooting.

Also, this entire block (env check → lazy import → ensure → delete → upsert) is duplicated nearly verbatim at lines 1404–1435 in process_file_with_smart_reindexing. Consider extracting a shared helper (e.g., _sync_graph_edges_best_effort(client, collection, file_path, repo, calls, imports)) to keep both call sites in sync and reduce copy-paste drift.

♻️ Proposed: extract helper and add minimal logging

Add a helper near the top of the file (after imports):

def _sync_graph_edges_best_effort(
    client: QdrantClient,
    collection: str,
    file_path: str,
    repo: str | None,
    calls: list[str] | None,
    imports: list[str] | None,
) -> None:
    """Best-effort sync of file-level graph edges. Safe to skip on failure."""
    enabled = str(os.environ.get("GRAPH_EDGES_ENABLE", "1") or "").strip().lower() in {
        "1", "true", "yes", "on",
    }
    if not enabled:
        return
    try:
        from scripts.ingest.graph_edges import (
            delete_edges_by_path,
            ensure_graph_collection,
            upsert_file_edges,
        )
        ensure_graph_collection(client, collection)
        delete_edges_by_path(client, collection, caller_path=file_path, repo=repo)
        upsert_file_edges(client, collection, caller_path=file_path, repo=repo, calls=calls, imports=imports)
    except Exception as exc:
        try:
            print(f"[graph_edges] best-effort sync failed for {file_path}: {exc}")
        except Exception:
            pass

Then replace both blocks:

-        # Optional: materialize file-level graph edges in a companion `<collection>_graph` store.
-        # This is an accelerator for symbol_graph callers/importers and is safe to skip on failure.
-        try:
-            enabled = str(os.environ.get("GRAPH_EDGES_ENABLE", "1") or "").strip().lower() in {
-                "1",
-                "true",
-                "yes",
-                "on",
-            }
-            if enabled:
-                from scripts.ingest.graph_edges import (
-                    delete_edges_by_path as _delete_edges_by_path,
-                    ensure_graph_collection as _ensure_graph_collection,
-                    upsert_file_edges as _upsert_file_edges,
-                )
-
-                _ensure_graph_collection(client, collection)
-                # Important: delete stale edges for this file before upserting the new set.
-                _delete_edges_by_path(
-                    client,
-                    collection,
-                    caller_path=str(file_path),
-                    repo=repo_tag,
-                )
-                _upsert_file_edges(
-                    client,
-                    collection,
-                    caller_path=str(file_path),
-                    repo=repo_tag,
-                    calls=calls,
-                    imports=imports,
-                )
-        except Exception:
-            pass
+        _sync_graph_edges_best_effort(client, collection, str(file_path), repo_tag, calls, imports)

1404-1435: Duplicate graph-edge sync block — same concern as lines 656–689.

This is a near-verbatim copy of the graph-edge sync block in _index_single_file_inner. If you extract the helper suggested above, this becomes a one-liner too, and the two paths stay in lockstep automatically.

scripts/ingest_code.py (1)

215-232: Asymmetric fallback: graph_edges_backfill_tick = None vs. no-op functions for the other two.

This is a valid design choice (callers should explicitly check before invoking a periodic tick), but it creates a subtle API inconsistency in the façade. Any consumer calling graph_edges_backfill_tick(...) without a None check will get a TypeError at runtime when graph_edges is unavailable.

If this is intentional (forcing callers to guard the call), consider adding a brief docstring or inline comment explaining why the backfill tick uses None while the others use no-op stubs.

scripts/indexing_admin.py (2)

930-935: Best-effort graph companion cleanup — consider a debug log for production visibility.

The _graph suffix guard correctly prevents recursive deletion. The silent except Exception: pass is consistent with the best-effort pattern, but a brief log message (even at debug level) would help operators understand why graph data might persist after a collection delete, especially in cases where the Qdrant API returns a non-404 error (e.g., timeout, auth failure).

🔧 Minimal logging suggestion
         if not name.endswith("_graph"):
             try:
                 cli.delete_collection(collection_name=f"{name}_graph")
-            except Exception:
-                pass
+            except Exception as exc:
+                try:
+                    print(f"[indexing_admin] best-effort graph collection delete failed for {name}_graph: {exc}")
+                except Exception:
+                    pass

960-965: Same pattern as delete_collection_qdrant — same optional logging suggestion applies.

scripts/prune.py (1)

42-90: Consider reusing the shared _normalize_path function for consistency.

The path normalization logic in delete_graph_edges_by_path is nearly identical to graph_edges.py's _normalize_path helper, but there's a minor exception-handling divergence: if os.path.normpath fails, graph_edges.py still applies backslash replacement while prune.py returns the path unchanged. While this is unlikely to cause issues in practice, extracting _normalize_path into a shared utility module would eliminate duplication and guarantee identical behavior across both modules.

Comment thread scripts/mcp_impl/symbol_graph.py Outdated
@voarsh2 voarsh2 changed the title Mit base salvage symbol Implement graph edges collection support with admin and ingest updates Feb 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@scripts/collection_admin.py`:
- Around line 493-510: Module-level logger is missing causing NameError in the
except block; add a module logger by importing logging and creating logger =
logging.getLogger(__name__) near the top-level imports so logger.debug in the
best-effort graph copy block (around copy_collection_qdrant usage) can run
without raising; ensure the import and logger creation are placed at module
scope so all functions in scripts/collection_admin.py can use logger.
🧹 Nitpick comments (7)
scripts/mcp_impl/symbol_graph.py (1)

27-34: Consider expiring the missing-collection cache.

_MISSING_GRAPH_COLLECTIONS is a permanent negative cache, so long-lived processes won’t re-check if collections appear later. A TTL or manual reset hook would keep it fresh.

scripts/standalone_upload_client.py (1)

733-768: Consider sharing ignore logic with scripts/remote_upload_client.py.

_excluded_dirnames and _is_ignored_path mirror the other client; extracting a shared helper would reduce drift over time.

scripts/remote_upload_client.py (2)

523-562: Duplicated logic with standalone_upload_client.py.

Both _excluded_dirnames and _is_ignored_path are near-identical copies of the same methods in scripts/standalone_upload_client.py (lines 732–767). The comment on line 524 acknowledges this ("Keep in sync with standalone_upload_client exclusions"), but keeping them manually in sync is error-prone.

Consider extracting these into a shared utility module to avoid divergence.


556-561: Extensionless-file lookup is case-sensitive on the dict side.

Line 560 compares rel.name.lower() against the lowered keys in extensionless. However, idx.EXTENSIONLESS_FILES keys may already be lowercase by convention. If a key were ever added with mixed case, this set(…keys()) (without lowering each key) would silently miss it.

The standalone client at line 766 uses (EXTENSIONLESS_FILES or {}).keys() without lowering keys either. This is consistent but fragile.

Suggested defensive fix
         try:
-            extensionless = set((idx.EXTENSIONLESS_FILES or {}).keys())
+            extensionless = {k.lower() for k in (idx.EXTENSIONLESS_FILES or {}).keys()}
         except Exception:
             extensionless = set()
scripts/ingest/graph_edges.py (3)

67-149: Silently swallowed index-creation errors may hide configuration issues.

Line 141–142: the except Exception: pass when creating payload indexes means a persistent configuration issue (e.g., invalid field schema, Qdrant version mismatch) will never surface. Since this is called once per collection lifecycle, a logger.debug would be inexpensive and aids troubleshooting.

Suggested improvement
             try:
                 client.create_payload_index(
                     collection_name=graph_coll,
                     field_name=field,
                     field_schema=qmodels.PayloadSchemaType.KEYWORD,
                 )
-            except Exception:
-                pass
+            except Exception as exc:
+                logger.debug("Payload index creation for %s.%s skipped: %s", graph_coll, field, exc)

240-276: delete_edges_by_path returns 1 on success regardless of actual delete count.

The return value is typed -> int but always returns 1 on success and 0 on failure, not the number of edges deleted. This could be confusing for callers that expect an actual count. Consider renaming or documenting the return semantics clearly.


279-385: Backfill tick looks correct; minor note on lazy import time.

The incremental scroll + per-file upsert logic is clean. The retry backoff at lines 341–345 works correctly.

Line 343: import time is lazily imported inside the retry block. This is harmless (Python caches module imports) but unconventional. Consider moving it to the top-level imports for clarity.

Comment thread scripts/collection_admin.py
Implements a `debug` parameter for the repo search functionality.

When `debug=true`, includes verbose internal fields (like components, rerank_counters, and code_signals) in the search results.
When `debug=false` (default), strips these fields to reduce token consumption.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)
scripts/mcp_indexer_server.py (1)

1649-1681: ⚠️ Potential issue | 🟡 Minor

code_search doesn't forward debug (or output_format, repo).

code_search is documented as an "exact alias of repo_search" but its signature and forwarding call are missing debug, output_format, and repo parameters. Users calling code_search cannot access debug mode.

Proposed fix (debug only, matching this PR's scope)
 async def code_search(
     query: Any = None,
     ...
     compact: Any = None,
+    debug: Any = None,
     kwargs: Any = None,
 ) -> Dict[str, Any]:
     ...
     return await repo_search(
         ...
         compact=compact,
+        debug=debug,
         kwargs=kwargs,
     )
🤖 Fix all issues with AI agents
In `@scripts/mcp_impl/search.py`:
- Around line 494-498: The assignment to debug_raw is unused and should be
removed; in the function where debug is processed (the lines assigning debug_raw
= debug and debug = _to_bool(debug, False)), delete the debug_raw = debug
statement and keep only debug = _to_bool(debug, False) so the code no longer
defines an unused variable (referencing the debug_raw symbol and the _to_bool
call to locate the change).
- Around line 80-91: The function _strip_debug_fields currently ignores the
keep_paths parameter and always strips host_path/container_path because
_DEBUG_RESULT_FIELDS includes those keys; update _strip_debug_fields to respect
keep_paths by only removing host_path and container_path when keep_paths is
False (e.g., compute the set of fields to strip from _DEBUG_RESULT_FIELDS minus
{"host_path","container_path"} when keep_paths is True, or copy the item and pop
those keys conditionally), leaving other debug removals unchanged; ensure the
docstring stays consistent with the new behavior.

Comment thread scripts/mcp_impl/search.py
Comment thread scripts/mcp_impl/search.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@ctx-mcp-bridge/src/mcpServer.js`:
- Around line 81-96: The function dedupeResourceTemplates is incorrectly reading
uriTemplate from template.resourceTemplate.uriTemplate which is always undefined
for MCP ResourceTemplate objects; update the extraction to read
template.uriTemplate (and keep a safe typeof string check) so uri is correctly
derived from each ResourceTemplate, then the seen/skip logic will work and out
will contain deduplicated templates; reference the dedupeResourceTemplates
function and the templates/uriTemplate symbols when making the change.

Comment thread ctx-mcp-bridge/src/mcpServer.js
Adds a new "bundled" mode for the MCP bridge, allowing the extension to use a pre-packaged bridge binary.

The extension now attempts to use the bundled bridge if the `mcpBridgeMode` setting is set to "bundled". If not found, it falls back to external resolution.

Also rewords the "localOnly" setting description to reflect the addition of "mcpBridgeMode".
- Updates code search to accept additional parameters for debugging and output formatting.

- Caches the excluded directory names in the upload clients to improve performance and prevent unintended behavior from runtime changes to exclusion settings.

- Adds a fallback mechanism for Qdrant client connections in the admin copy collection function, ensuring resilience in case pooled client acquisition fails.
Ensures the user is prompted to create a virtual environment and install dependencies when both the initial Python environment and auto-detected environments fail to provide the required modules. Removes the conditional check that prevented the prompt when modules were missing from an auto-detected interpreter.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @.github/workflows/claude.yaml:
- Around line 12-13: The workflow uses the pull_request_target event combined
with checking out the PR head (github.event.pull_request.head.sha), which lets
untrusted PR code run with elevated permissions; change the workflow event to
pull_request or, if pull_request_target is required, update the checkout step to
use the base branch (github.event.pull_request.base.sha) instead of the PR head,
or add an explicit human-approval job that gates any steps requiring secrets;
search for the token usage and the checkout action (references:
pull_request_target, github.event.pull_request.head.sha,
github.event.pull_request.base.sha) and implement one of these safer
alternatives.
- Around line 18-27: The conditional in the workflow only checks
github.event.comment.author_association, so jobs triggered by
pull_request_target events (where the PR body contains "@claude") never pass
because there is no comment object; update the if expression to validate author
association for the correct event payloads by adding checks for
github.event.pull_request.author_association and
github.event.review.author_association (in addition to
github.event.comment.author_association and
github.event.issue.author_association) or conditionally select the association
based on event type (e.g., check github.event_name == 'pull_request' ?
github.event.pull_request.author_association :
github.event.comment.author_association) while preserving the existing
sender.type == 'User' and allowed associations (OWNER, MEMBER, COLLABORATOR) so
PR-open/synchronize events will run when `@claude` is in the PR body.

In `@scripts/remote_upload_client.py`:
- Around line 1419-1428: The watcher currently only checks idx.CODE_EXTS using
the file suffix, so extensionless code files (e.g., Dockerfile, Makefile,
.gitignore) are skipped; change the filter in both the src_path and dest_path
branches to consult CODE_EXTS using the file suffix when present and fall back
to the filename when suffix is empty — e.g., compute key =
src_path.suffix.lower() or src_path.name when suffix is empty and use
idx.CODE_EXTS.get(key, "unknown") != "unknown" while preserving the existing
self.client._is_ignored_path checks (apply same change for dest_path handling).
🧹 Nitpick comments (7)
vscode-extension/build/build.sh (1)

83-95: Guard missing bin/src to avoid hard build failures.

With set -e, missing bin or src will abort packaging even though the bridge source exists. Consider making these copies conditional (mirroring the node_modules behavior) to keep bundling best‑effort.

🛠️ Suggested hardening
 if [[ -d "$BRIDGE_SRC" && -f "$BRIDGE_SRC/package.json" ]]; then
     echo "Bundling MCP bridge npm package into staged extension..."
     mkdir -p "$STAGE_DIR/$BRIDGE_DIR"
-    cp -a "$BRIDGE_SRC/bin" "$STAGE_DIR/$BRIDGE_DIR/"
-    cp -a "$BRIDGE_SRC/src" "$STAGE_DIR/$BRIDGE_DIR/"
+    if [[ -e "$BRIDGE_SRC/bin" ]]; then
+        cp -a "$BRIDGE_SRC/bin" "$STAGE_DIR/$BRIDGE_DIR/"
+    else
+        echo "Warning: Bridge bin not found; skipping."
+    fi
+    if [[ -d "$BRIDGE_SRC/src" ]]; then
+        cp -a "$BRIDGE_SRC/src" "$STAGE_DIR/$BRIDGE_DIR/"
+    else
+        echo "Warning: Bridge src not found; skipping."
+    fi
     cp "$BRIDGE_SRC/package.json" "$STAGE_DIR/$BRIDGE_DIR/"
scripts/mcp_indexer_server.py (1)

1193-1226: Consider forwarding the debug parameter in repo_search_compat for consistency.

The new debug parameter added to repo_search is not forwarded through repo_search_compat. Clients using this compatibility wrapper won't be able to enable debug mode.

♻️ Proposed fix to forward debug parameter
         forward = {
             "query": query,
             "limit": limit,
             "per_path": args.get("per_path"),
             "include_snippet": args.get("include_snippet"),
             "context_lines": args.get("context_lines"),
             "rerank_enabled": args.get("rerank_enabled"),
             "rerank_top_n": args.get("rerank_top_n"),
             "rerank_return_m": args.get("rerank_return_m"),
             "rerank_timeout_ms": args.get("rerank_timeout_ms"),
             "highlight_snippet": args.get("highlight_snippet"),
             "collection": args.get("collection"),
             "session": args.get("session"),
             "workspace_path": args.get("workspace_path"),
             "language": args.get("language"),
             "under": args.get("under"),
             "kind": args.get("kind"),
             "symbol": args.get("symbol"),
             "path_regex": args.get("path_regex"),
             "path_glob": args.get("path_glob"),
             "not_glob": args.get("not_glob"),
             "ext": args.get("ext"),
             "not_": not_value,
             "case": args.get("case"),
             "compact": args.get("compact"),
+            "debug": args.get("debug"),
             "mode": args.get("mode"),
             "repo": args.get("repo"),  # Cross-codebase isolation
             "output_format": args.get("output_format"),  # "json" or "toon"
             # Alias passthroughs captured by repo_search(**kwargs)
             "queries": queries,
             "q": args.get("q"),
             "text": args.get("text"),
             "top_k": args.get("top_k"),
         }
scripts/upload_service.py (1)

1245-1286: Best-effort graph copy detection logic is acceptable but complex.

The nested try-except structure correctly implements a fallback chain (pooled client → direct client → default to "0"), which is appropriate for a non-critical "best-effort" feature. A few observations:

  1. Line 1271: The or 5 is redundant since the default is already "5".
  2. The try-except-pass on cli.close() (lines 1279-1282) is acceptable for cleanup code where logging would add noise.

Consider extracting the graph-existence check into a helper function if this pattern is reused elsewhere, but it's acceptable as-is for a single use case.

♻️ Minor cleanup for redundant default
                     cli = QdrantClient(
                         url=QDRANT_URL,
                         api_key=os.environ.get("QDRANT_API_KEY"),
-                        timeout=float(os.environ.get("QDRANT_TIMEOUT", "5") or 5),
+                        timeout=float(os.environ.get("QDRANT_TIMEOUT", "5")),
                     )
vscode-extension/context-engine-uploader/mcp_bridge.js (1)

99-107: Potential mismatch between reported kind and actual source.

When mode === 'bundled' but the bundled binary is not found, findLocalBridgeBin() falls back to external resolution (configured path or env override). If an external binary exists, binPath will be set, but kind would still be reported as 'bundled' because line 106 only checks the mode, not the actual source of the binary.

Consider tracking whether the path came from the bundled source:

♻️ Proposed fix to accurately report the binary source
 function resolveBridgeCliInvocation() {
-  const binPath = findLocalBridgeBin();
   const mode = getBridgeMode();
+  const bundledBin = mode === 'bundled' ? findBundledBridgeBin() : undefined;
+  const binPath = bundledBin || findLocalBridgeBin();
+  const isBundled = !!bundledBin;
   if (binPath) {
     return {
       command: 'node',
       args: [binPath],
-      kind: mode === 'bundled' ? 'bundled' : 'local'
+      kind: isBundled ? 'bundled' : 'local'
     };
   }

Note: This would require refactoring findLocalBridgeBin() to skip bundled resolution when called from here, or extracting the bundled check.

.github/workflows/claude.yaml (2)

44-54: Redundant configuration: settings file and inline settings parameter both defined.

The Claude settings are configured twice:

  1. Lines 44-54: Written to /home/runner/.claude/settings.json
  2. Line 64: Passed via settings parameter

This duplication is unnecessary and could cause confusion if they diverge. Pick one approach.

♻️ Proposed fix: Remove the redundant settings file step
-      - name: Create Claude settings file
-        run: |
-          mkdir -p /home/runner/.claude
-          cat > /home/runner/.claude/settings.json << 'EOF'
-          {
-            "env": {
-              "ANTHROPIC_BASE_URL": "https://api.z.ai/api/anthropic",
-              "ANTHROPIC_AUTH_TOKEN": "${{ secrets.CUSTOM_ENDPOINT_API_KEY }}"
-            }
-          }
-          EOF
-
       - name: Run Claude Code

Also applies to: 64-64


39-39: Consider pinning GitHub Actions to specific commit SHAs for enhanced supply chain security.

This workflow has write permissions (contents: write, pull-requests: write, issues: write) and accesses secrets (CUSTOM_ENDPOINT_API_KEY), making it a higher-value target. Pinning actions/checkout@v4 and anthropics/claude-code-action@v1 to full commit SHAs prevents potential supply chain attacks if an action repository is compromised. Major version tags can change without notice, whereas commit SHAs are immutable.

vscode-extension/context-engine-uploader/python_env.js (1)

363-365: Redundant cache check.

This cache check will never return true at this point in the flow—venvPython was just resolved and the cache is only populated on line 368 after checkPythonDeps succeeds. This block is effectively dead code.

🧹 Proposed cleanup
     setPythonOverridePath(venvPython);
     log(`Using private venv interpreter: ${getPythonOverridePath()}`);
     const venvKey = cacheKey(venvPython, workingDirectory);
-    if (depCheckCache.get(venvKey)) {
-        return true;
-    }
     const finalOk = await checkPythonDeps(venvPython, workingDirectory, { showInterpreterError: true });
     if (finalOk) {
         depCheckCache.set(venvKey, true);

Comment on lines +12 to +13
pull_request_target:
types: [opened, synchronize]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: pull_request_target with PR head checkout creates a "pwn request" vulnerability.

Using pull_request_target grants write permissions and access to secrets, but checking out github.event.pull_request.head.sha (the untrusted PR code) allows malicious PRs to execute arbitrary code with those elevated privileges. An attacker could modify workflow files or exfiltrate secrets.

Safer alternatives:

  1. Use pull_request event instead (runs in PR context without secrets access)
  2. If pull_request_target is required, only checkout the base branch, not the PR head
  3. Add an explicit approval step before running on external PRs
🔒 Recommended fix: Use pull_request event or avoid checking out PR head
-  pull_request_target:
-    types: [opened, synchronize]
+  pull_request:
+    types: [opened, synchronize]

Or if you need secrets access, don't checkout PR head:

      - name: Checkout repository
        uses: actions/checkout@v4
-        with:
-          # This correctly checks out the PR's head commit for pull_request_target events.
-          ref: ${{ github.event.pull_request.head.sha }}

Also applies to: 38-42

🤖 Prompt for AI Agents
In @.github/workflows/claude.yaml around lines 12 - 13, The workflow uses the
pull_request_target event combined with checking out the PR head
(github.event.pull_request.head.sha), which lets untrusted PR code run with
elevated permissions; change the workflow event to pull_request or, if
pull_request_target is required, update the checkout step to use the base branch
(github.event.pull_request.base.sha) instead of the PR head, or add an explicit
human-approval job that gates any steps requiring secrets; search for the token
usage and the checkout action (references: pull_request_target,
github.event.pull_request.head.sha, github.event.pull_request.base.sha) and
implement one of these safer alternatives.

Comment on lines +18 to +27
if: >
(contains(github.event.comment.body, '@claude') ||
contains(github.event.review.body, '@claude') ||
contains(github.event.issue.body, '@claude') ||
contains(github.event.pull_request.body, '@claude')) &&
(github.event.sender.type == 'User' && (
github.event.comment.author_association == 'OWNER' ||
github.event.comment.author_association == 'MEMBER' ||
github.event.comment.author_association == 'COLLABORATOR'
))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Condition logic is incomplete for pull_request_target events.

The author_association check only references github.event.comment.author_association, which is undefined for pull_request_target opened/synchronize events (there's no comment). This causes the job to never run for those triggers even when @claude is in the PR body.

If you intend to support PR-triggered runs, you need to check associations conditionally:

🛠️ Proposed fix to handle different event types
     if: >
       (contains(github.event.comment.body, '@claude') ||
       contains(github.event.review.body, '@claude') ||
       contains(github.event.issue.body, '@claude') ||
       contains(github.event.pull_request.body, '@claude')) &&
-      (github.event.sender.type == 'User' && (
-        github.event.comment.author_association == 'OWNER' ||
-        github.event.comment.author_association == 'MEMBER' ||
-        github.event.comment.author_association == 'COLLABORATOR'
+      github.event.sender.type == 'User' && (
+        github.event.comment.author_association == 'OWNER' ||
+        github.event.comment.author_association == 'MEMBER' ||
+        github.event.comment.author_association == 'COLLABORATOR' ||
+        github.event.review.author_association == 'OWNER' ||
+        github.event.review.author_association == 'MEMBER' ||
+        github.event.review.author_association == 'COLLABORATOR' ||
+        github.event.issue.author_association == 'OWNER' ||
+        github.event.issue.author_association == 'MEMBER' ||
+        github.event.issue.author_association == 'COLLABORATOR' ||
+        github.event.pull_request.author_association == 'OWNER' ||
+        github.event.pull_request.author_association == 'MEMBER' ||
+        github.event.pull_request.author_association == 'COLLABORATOR'
-      ))
+      )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if: >
(contains(github.event.comment.body, '@claude') ||
contains(github.event.review.body, '@claude') ||
contains(github.event.issue.body, '@claude') ||
contains(github.event.pull_request.body, '@claude')) &&
(github.event.sender.type == 'User' && (
github.event.comment.author_association == 'OWNER' ||
github.event.comment.author_association == 'MEMBER' ||
github.event.comment.author_association == 'COLLABORATOR'
))
if: >
(contains(github.event.comment.body, '@claude') ||
contains(github.event.review.body, '@claude') ||
contains(github.event.issue.body, '@claude') ||
contains(github.event.pull_request.body, '@claude')) &&
github.event.sender.type == 'User' && (
github.event.comment.author_association == 'OWNER' ||
github.event.comment.author_association == 'MEMBER' ||
github.event.comment.author_association == 'COLLABORATOR' ||
github.event.review.author_association == 'OWNER' ||
github.event.review.author_association == 'MEMBER' ||
github.event.review.author_association == 'COLLABORATOR' ||
github.event.issue.author_association == 'OWNER' ||
github.event.issue.author_association == 'MEMBER' ||
github.event.issue.author_association == 'COLLABORATOR' ||
github.event.pull_request.author_association == 'OWNER' ||
github.event.pull_request.author_association == 'MEMBER' ||
github.event.pull_request.author_association == 'COLLABORATOR'
)
🤖 Prompt for AI Agents
In @.github/workflows/claude.yaml around lines 18 - 27, The conditional in the
workflow only checks github.event.comment.author_association, so jobs triggered
by pull_request_target events (where the PR body contains "@claude") never pass
because there is no comment object; update the if expression to validate author
association for the correct event payloads by adding checks for
github.event.pull_request.author_association and
github.event.review.author_association (in addition to
github.event.comment.author_association and
github.event.issue.author_association) or conditionally select the association
based on event type (e.g., check github.event_name == 'pull_request' ?
github.event.pull_request.author_association :
github.event.comment.author_association) while preserving the existing
sender.type == 'User' and allowed associations (OWNER, MEMBER, COLLABORATOR) so
PR-open/synchronize events will run when `@claude` is in the PR body.

Comment thread scripts/remote_upload_client.py
@voarsh2

voarsh2 commented Mar 2, 2026

Copy link
Copy Markdown
Owner Author

@claude can you see the outstanding review items from coderabbit?

Repository owner deleted a comment from claude Bot Mar 2, 2026
@claude

claude Bot commented Mar 2, 2026

Copy link
Copy Markdown

Claude finished @voarsh2's task in 1m 35s —— View job


Outstanding CodeRabbit Review Items

Yes, I can see the CodeRabbit review comments. Here's a summary of the outstanding items that have not been addressed:


🔴 CRITICAL - Security Vulnerability

.github/workflows/claude.yaml (lines 12-13, 38-42)

The workflow uses pull_request_target with elevated permissions and checks out the PR head commit (github.event.pull_request.head.sha), creating a "pwn request" vulnerability. This allows untrusted PR code to run with write permissions and secrets access.

Recommended fix: Change to pull_request event OR checkout only the base branch.

⚠️ I cannot modify workflow files due to GitHub App permissions restrictions. You'll need to fix this manually.


🟠 MAJOR - Workflow Condition Logic

.github/workflows/claude.yaml (lines 18-27)

The condition only checks github.event.comment.author_association, which is undefined for pull_request_target events. This causes jobs triggered by PR body @claude mentions to never run.

Suggested fix: Add checks for github.event.pull_request.author_association, github.event.review.author_association, and github.event.issue.author_association.

Also cannot be fixed by me due to workflow permission restrictions.


🟠 MAJOR - Watcher Skips Extensionless Files

scripts/remote_upload_client.py (lines 1421, 1427)

The watchdog event filter only checks idx.CODE_EXTS using file suffix, so extensionless code files (e.g., Dockerfile, Makefile, .gitignore) are skipped in watch mode.

Fix this →


✅ Already Addressed (in previous commits)

  • scripts/mcp_impl/symbol_graph.py - Loop variable closure (commit 7961a72)
  • scripts/collection_admin.py - Missing logger (commits 2476d6c to b43054b)
  • scripts/mcp_impl/search.py - keep_paths parameter and debug_raw variable (commit 2f14eff)
  • ctx-mcp-bridge/src/mcpServer.js - Incorrect dedupeResourceTemplates property path (commit 2f14eff)

🧹 Nitpick Items (Optional Improvements)

CodeRabbit also noted several minor code quality improvements (caching excluded dirnames, reusing shared helpers, adding debug logs, etc.). These are low-priority and don't affect functionality.


Summary

Priority Issue Can I Fix?
🔴 Critical Workflow security vulnerability ❌ No (workflow permissions)
🟠 Major Workflow condition logic ❌ No (workflow permissions)
🟠 Major Watcher extensionless files ✅ Yes
🟡 Minor Nitpicks (15 items) ✅ Yes

Would you like me to fix the watcher issue for extensionless files?


Task Checklist

Repository owner deleted a comment from claude Bot Mar 2, 2026
@voarsh2

voarsh2 commented Mar 2, 2026

Copy link
Copy Markdown
Owner Author

@claude by design, we opted only to watch specific file extensions... so that issue is moot.

@claude

claude Bot commented Mar 2, 2026

Copy link
Copy Markdown

Claude finished @voarsh2's task in 23s —— View job


Response to Design Clarification

Understood - thank you for the clarification. You're absolutely right that this is an intentional design decision, not a bug.

The watcher in remote_upload_client.py (lines 1421-1427) is designed to only watch specific file extensions defined in idx.CODE_EXTS. This means:

  • Extensionless code files (Dockerfile, Makefile, .gitignore, etc.) are intentionally excluded from watchdog-based watching
  • These files can still be indexed through other means (explicit upload, CLI-based ingestion, etc.)
  • The behavior is working as designed

This is a reasonable architectural choice:

  • Reduced filesystem overhead: Fewer events to process in watch mode
  • Clear boundary: Watch mode focuses on the most common code files
  • Alternative paths available: Extensionless files can still be indexed via other flows

Summary

The CodeRabbit review item about "watcher skipping extensionless files" can be dismissed - this is intentional behavior by design, not a defect requiring a fix.

Branch: mit-base-salvage-symbol
View job

voarsh added 4 commits March 2, 2026 11:05
Ensures that when a user's session ID changes, such as after an authentication event, the `defaultsPayload` is updated with the new session ID and re-sent to the indexer and memory clients. This keeps backend services synchronized with the current active session.

Also adds the session ID to the defaults payload during initial setup.
Extract repeated session defaults synchronization into ensureRemoteDefaults
function with deduplication tracking via lastDefaultsSyncedSessionId to
prevent redundant sync operations across session lifecycle events.
Refactor path scoping logic into shared `scripts/path_scope` module and
change `under` parameter semantics from exact path_prefix match to recursive subtree filtering. This allows `under="scripts"` to match all files under scripts/** rather than requiring exact prefix equality.

- Add `scripts/path_scope.py` with `normalize_under`, `metadata_matches_under`,
  and `path_matches_under` helpers for consistent path scoping
- Update hybrid_search, expand.py, symbol_graph, and rerank_tools/local.py to use shared path scoping module with client-side post-filtering
- Remove server-side path_prefix equality filters in favor of recursive client-side matching against multiple path forms (repo_rel_path, host_path, container_path, etc.)
- Add overfetch multiplier for rerank when `under` is specified to ensure sufficient candidates before client-side filtering
- Update docstrings to clarify `under` as recursive workspace subtree filter
- Add comprehensive tests for new path scoping behavior
- Modernize test asyncio usage from deprecated get_event_loop() to asyncio.run()
…ging

- Replace blocking subprocess.run with ThreadPoolExecutor for non-blocking
  git history ingestion in watch processor
- Add comprehensive progress logging with metrics (prepared, persisted,
  failures) in ingest_history.py
- Add skip-reason logging for git history collection in upload clients
- Support configurable timeout via WATCH_GIT_HISTORY_TIMEOUT_SECONDS
- Track in-flight manifests to prevent duplicate processing
- Stream stdout/stderr from ingestion subprocess with tail capture
voarsh added 30 commits March 9, 2026 22:45
Introduce withTransientRetry helper that wraps operations with automatic retry on transient errors. Apply to listMemoryTools, listResourcesSafe, listResourceTemplatesSafe, and tools/list request handler to improve reliability when remote MCP servers experience temporary failures.

Also replace refreshSessionAndSyncDefaults calls with initializeRemoteClients and ensureRemoteDefaults for clearer initialization semantics in request handlers.
Add GitHub Actions workflow for automated CoSQA search benchmarks with:
- Scheduled daily runs and PR triggers for search-related paths
- Configurable hybrid gate enforcement to catch regressions
- Qdrant service container for isolated benchmark execution
- Artifact upload for results and summaries

Add run_search_matrix.sh orchestration script supporting multiple
profiles (smoke/quick/full) and run sets (pr/knobs/nightly/full)
with comprehensive metric collection and comparison.

Fix runner.py to correctly extract CoSQA code IDs from synthetic
filenames and disable MCP auth during benchmark execution.
- support REPO_SEARCH_DEFAULT_MODE for dense-focused repo_search
- forward dense search filters for kind, symbol, ext, under, repo, and per-path caps
- keep hybrid/rerank tests explicit when exercising non-dense behavior
- clarify PSEUDO_BACKFILL_ENABLED vs PSEUDO_DEFER_TO_WORKER semantics
- avoid silently dropping inline pseudo generation when worker backfill is disabled
- reuse graph-edge deletion helpers from prune and watcher delete verification
- normalize graph caller paths consistently for write/delete/verify flows
- prune orphan _graph edges whose base-code points no longer exist
- include _graph orphan checks in watcher consistency audit summaries
- harden journal replay and force-upsert verification around cached-hash fast paths
- add regression coverage for dense mode, graph deletes, prune, and watcher consistency
Adds a robust session recovery mechanism for all MCP list operations (tools, resources, resource templates).

Upon detection of a session-related error, the system now attempts to re-initialize remote clients and then retries the failed list request. This significantly improves the bridge's resilience against transient session issues, reducing user-facing errors.

Updates existing list functions to accept a client getter and an `onSessionError` callback to facilitate this recovery logic. Also expands session error detection to include requests received before full client initialization.
Remove the experimental MCP router stack and redundant search wrappers:
code_search, info_request, search_tests_for, search_config_for,
search_callers_for, and search_importers_for. Consolidate focused lookup behavior into repo_search(profile=tests|config|code) instead of exposing separate heuristic tools.

Keep the small MCP HTTP transport helper needed by ctx/context_search, but
drop router planning, batching, scratchpad, eval, benchmark, and router tests.
Update docs, skills, benchmark references, config examples, and Kubernetes
config to match the smaller tool surface.

Also harden ctx after the router removal by parsing structured MCP responses, forwarding per_path, routing GLM rewrites through the GLM adapter, and adding basic ctx CLI tests.
Keep TOON as an agent-readable text render without replacing the structured results arrays returned by search tools. repo_search, context_search, and pattern search now attach TOON output under text while preserving results for machine callers.

Tighten TOON tests around the new response contract, clean up formatter imports, remove stale text-only assumptions, and update the documented token savings to match measured compact-JSON comparisons.
Eliminates complex Python virtual environment (venv) creation and dependency installation, making extension setup significantly easier for users.

Removes the `python_env.js` module and replaces its functionality with a streamlined Python 3 interpreter detection mechanism that probes common system paths.

The extension now ships with bundled Python libraries (`python_libs`), which are automatically added to the `PYTHONPATH` when running the upload client. This removes the need for users to manually install Python dependencies via `pip`.

Enhances the robustness of the HTTP MCP bridge process by ensuring clean state management and cancelling pending configuration refreshes upon bridge shutdown or error.

Updates the README and package description to reflect these changes.
Pins qdrant-client to 1.15.1 and Qdrant server images to v1.15.4 across Docker, Kubernetes, CI, and testcontainers. Updates docs and adds regression tests to ensure the pins and supported client API do not drift.
Runs the existing init maintenance sequence periodically from the long-lived watcher with a shared cross-process lock, defaulting to every 120 minutes with no immediate startup run. Reuses the same locked path from the init job, adds env controls and docs, and covers scheduling/sequence behavior with focused tests.
Remove compatibility shims and fallback import paths now that the app runs as a complete package. Update hybrid, MCP, rerank, benchmark, upload, and indexing code to import the real modules directly, and delete the old top-level shim files that kept stale import paths alive.

Run Docker, Compose, Kubernetes, Makefile, and subprocess entrypoints with python -m scripts.* from /app so package imports resolve consistently in containers. Normalize script permissions in images so non-root workers can load the packaged scripts, including watcher init maintenance modules.

Tighten collection resolution for upload-managed multi-repo mode. Stop treating /work as a repository identity, avoid deriving global-collection from the workspace root, and keep multi-repo indexing paths on the configured collection unless a real repo slug is being processed.

Scope health checks to COLLECTION_NAME by default so stale or unrelated Qdrant collections do not fail init payload. Make missing named vectors report cleanly instead of raising KeyError.

Clean up tests after removing import fallbacks by making env/module state
explicit, avoiding ambient .env leakage, isolating watcher sleep monkeypatches,
and adding regressions for health-check targeting and /work collection resolution.
Upload-managed repos are server-owned, so stop remote upload clients from
generating or sending collection_name in config, manifests, plan/apply
payloads, or bundle uploads. Resolve upload collection identity on the server
from source_path/logical repo state instead, and ignore stale client-supplied
collection routing.

Also stop the memory MCP server from eagerly creating the default collection on
startup. In multi-repo mode, treat codebase as the single built-in default and
require an explicit/session collection for memory tools instead of silently
creating codebase. Keep codebase as the one out-of-box collection for
single-collection/bindmount flows.

Trim old placeholder collection names from workspace/indexer logic and docs,
and update focused coverage for upload-managed collection routing and
collection default handling.
Launch the watcher git-history ingest worker with `python -m
scripts.ingest_history` from the app root instead of directly invoking the
script path. This keeps package imports working in containers where `/app` is
the import root and fixes `ModuleNotFoundError: No module named 'scripts'`.

Add a focused regression test for the subprocess command, cwd, and collection
env propagation.
Treat configured workspace roots as roots in multi-repo journal resolution so a watched content root like /work is not misclassified as a repo when CTXCE_METADATA_ROOT points elsewhere.

Also scan the explicit metadata root's .codebase/repos directory when listing pending journal entries from a different WATCH_ROOT, allowing the watcher to drain and clear upload-created journal entries in dogfood split-root setups.

Wire docker compose watcher/upload-service defaults to CTXCE_METADATA_ROOT=/work so local compose keeps producer and consumer metadata state aligned while still allowing dogfood overrides.

Add a regression test for split watch/metadata roots, including journal clear after marking an entry done.
Log aggregate watcher journal backlog and queue stats on a throttle so drain progress is visible without per-path spam.

Add an admin ACL collection action to clear the mapped repo journal through the existing workspace_state journal helper, with redirect feedback and focused coverage.

Retarget stale Makefile rerank commands to the package module entrypoints removed from the old script shims.
Default journal draining on for multi-repo upload-managed mode and filesystem events on for single-repo bindmount mode, with explicit WATCH_JOURNAL_DRAIN_ENABLED and WATCH_FS_EVENTS_ENABLED overrides.

Keep WATCH_USE_POLLING as the Kubernetes/shared-filesystem observer backend knob and wire it through the Kubernetes watcher config.

Verified with full pytest suite: 606 passed, 2 skipped in 594.15s.
Remove broad package facade imports from rerank, ingest, hybrid, and MCP
test paths so unit tests exercise concrete modules instead of pulling in
Qdrant, FastEmbed, OpenAI, and server entrypoints by accident.

Keep expensive dependencies behind their real adapter/use boundaries:
Qdrant model namespaces are lazy only at Qdrant-facing edges, rerank
optional backends load only when selected, and package __init__ files no
longer bulk-import feature islands.

Retarget search/context/upload tests toward implementation helpers with
explicit fakes, make TTL and async upload tests deterministic without
sleep/poll waits, and isolate smart reindex tests from pseudo/pattern/sparse
side effects. Mark the real Qdrant pattern-search check as integration so
the default suite does not pay for a service probe.

Default pytest now runs in about 21s with integration tests deselected.
Removes the self-supervised recursive reranker and its training stack so ranking no longer depends on opaque model-generated feedback.

Keeps filename boosting available through the normal hybrid-search path while eliminating unused neural state, persistence, and evaluation code.
Replaces structural pattern retrieval and implicit model supervision with explicit relevance feedback that can be inspected, persisted, and applied safely.

Adds stable result identity, recent-result metadata, rating capture, per-collection aggregation, positive-target recall, graph-based caller recall, and bounded ranking boosts.
Adds symbol-aware metadata and conservative reconciliation so feedback survives ordinary code evolution without attaching to ambiguous targets.

Removes the remaining indexing-time pattern-vector paths while recording file and symbol content hashes needed for rename and split detection.
Updates the compose stack to run explicit feedback training instead of the retired learning worker.

Shares feedback weights with search and indexing services, exposes trainer polling controls, and removes obsolete learning and pattern-vector configuration.
Documents explicit ratings, stable target identity, conservative reindex migration, and soft feedback-based recall so operators can understand the new retrieval behavior.

Removes configuration and architecture guidance for the retired learning reranker and structural pattern search.
Adds coverage for the end-to-end feedback lifecycle and the identity guarantees that make ratings durable across processes and reindex operations.

Updates integration-test discovery and Qdrant selection so the suite can run explicitly against CI, a compose stack, or isolated testcontainers.
Restores the test module's valid import separator so the file remains syntactically correct and pytest can collect it normally.
Removes the retired structural-pattern and self-supervised reranking surfaces so the project reflects the supported retrieval and relevance-feedback architecture.

Updates documentation, benchmarks, deployment configuration, tests, and collection setup to prevent references to deleted vectors, workers, and evaluation paths.
Makes feedback targets stable across path representations and qualified-symbol changes, while ensuring recalled results obey the same filters as ordinary search.

Preserves useful ratings through renames and prevents recalled candidates from being lost when the public result limit is applied.
Makes watcher journal replay observable and resource-bounded so large backlogs do not overwhelm indexing or hide processing state.

Adds batched status persistence, retry summaries, safer status normalization, and focused coverage for multi-repository operation.
Makes local upload state reflect the content actually accepted by the server and prevents stale or cross-workspace cache data from suppressing changes.

Reuses content hashes within each scan, validates files before finalizing them, safely retries timed-out requests, and improves diagnostics for planned work.
Prevents already-applied files and moves from being treated as failures during retries, while retaining sequence integrity when any replica remains inconsistent.

Adds plan diagnostics, validates target files rather than directories, preserves canonical replicas, and exposes partial failures accurately to clients.
Ensures batch pseudo-description generation runs only when pseudo descriptions are explicitly enabled, keeping sequential and smart reindex paths consistent.

Normalizes concurrency parsing and adds regression coverage for disabled generation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants