refactor(group): address Copilot review feedback on #796

Six fixes suggested by the Copilot AI review:

1. **`normalizeHttpPath` root-path edge case** — stripping trailing
   slashes on the input `/` produced an empty string, yielding
   malformed contract ids like `http::GET::`. Now preserves `/` for
   the root handler/fetch case.

2. **Dedupe `scanFiles` call** — `extract()` was globbing the
   source-scan file list twice (once for the provider fallback, once
   for the consumer fallback). Moved to a single lazy call that
   memoizes the result for the rest of the method.

3. **HTTP `scanFiles` now ignores `**/vendor/**`** — every other
   extractor's glob already ignored vendored sources; the HTTP one
   didn't. Fixed for consistency.

4. **`loadPackageDefinition` check is now structural** — was calling
   `tree.rootNode.text.includes('loadPackageDefinition')` which forces
   materialization of the entire file text from the parse tree
   (expensive on large files). Replaced with a dedicated compiled
   query on `(call_expression function: [(identifier) | (member_expression)])`
   so the check stays in the AST domain.

5. **`grpc-extractor.ts` header docstring updated** — still claimed
   ".proto parsing is not tree-sitter-based because no grammar is
   installed". Now describes the actual behaviour: tree-sitter when
   `tree-sitter-proto` is available (optionalDependency), manual
   fallback otherwise.

6. **Eliminated the double proto file parse on the fallback path** —
   `buildProtoContext` already globs + parses every `.proto` file to
   build `servicesByName`. On the `!hasProtoPlugin` branch the
   extractor was globbing + parsing again via the now-removed
   `parseProtoFile` helper. The fallback branch now iterates the map
   that `buildProtoContext` already produced to emit provider
   contracts directly — single pass per proto file.

## Tests

- `topic-extractor.test.ts` — 30/30 pass
- `http-route-extractor.test.ts` — 18/18 pass
- `grpc-extractor.test.ts` — 43/43 pass
- `manifest-extractor.test.ts` — 8/8 pass
- `npx tsc -p tsconfig.json --noEmit` clean

Co-authored-by: Claude <noreply@anthropic.com>

gitnexus/src/core/group/extractors/grpc-extractor.ts

@@ -17,21 +17,22 @@ import {
  *
  * Two parts:
  *
- * 1. **`.proto` parsing** — done in-process by a small string-sanitizing
- *    parser (see `stripProtoCommentsAndStrings` + `extractServiceBlocks`
- *    below). NOT tree-sitter-based because no `tree-sitter-proto`
- *    grammar is installed in the repo. The parser preserves offsets so
+ * 1. **`.proto` parsing** — tree-sitter when `tree-sitter-proto` is
+ *    installed (optionalDependency vendored in `vendor/tree-sitter-proto/`),
+ *    via the `.proto` entry in `grpc-patterns/` and `hasProtoPlugin`.
+ *    When the grammar isn't available (platform incompatibility, native
+ *    build failure) the orchestrator falls back to the in-process
+ *    string-sanitizing parser defined below (`stripProtoCommentsAndStrings`
+ *    + `extractServiceBlocks`). The fallback preserves offsets so any
  *    downstream regex scans run against a sanitized copy without
- *    affecting the line numbers of the original. Kept as a pragmatic
- *    exception to the "no regex in extractors" rule until / unless
- *    maintainers want to add a proto grammar.
+ *    affecting line numbers of the original.
  *
  * 2. **Source-scan providers / consumers** — delegated to per-language
  *    plugins in `./grpc-patterns/`. The orchestrator imports NO
  *    tree-sitter grammars or query strings — each plugin owns its own.
  */
 
-// ─── .proto parsing (not tree-sitter) ────────────────────────────────
+// ─── .proto fallback parser (used only when tree-sitter-proto is absent) ───
 
 function readSafe(repoPath: string, rel: string): string | null {
   const abs = path.resolve(repoPath, rel);
@@ -372,23 +373,29 @@ export class GrpcExtractor implements ContractExtractor {
     // ─── Proto files — definitive provider source ─────────────────
     // When tree-sitter-proto is available, .proto files are handled by
     // the plugin loop below (they're in GRPC_SCAN_GLOB). Otherwise
-    // fall back to the manual string-sanitizing parser.
+    // emit provider contracts directly from the proto map that
+    // `buildProtoContext` already built — no second glob / parse pass.
     if (!hasProtoPlugin) {
-      const protoFiles = await glob('**/*.proto', {
-        cwd: repoPath,
-        ignore: ['**/node_modules/**', '**/.git/**', '**/vendor/**'],
-        nodir: true,
-      });
-      for (const rel of protoFiles) {
-        const content = readSafe(repoPath, rel);
-        if (content) {
-          out.push(
-            ...this.parseProtoFile(
-              content,
-              rel,
-              protoContext.packagesByProto.get(normalizeProtoPath(rel)) ?? '',
-            ),
-          );
+      for (const infos of protoMap.values()) {
+        for (const info of infos) {
+          for (const methodName of info.methods) {
+            const cid = contractId(info.package, info.serviceName, methodName);
+            out.push(
+              makeContract(
+                cid,
+                'provider',
+                info.protoPath,
+                `${info.serviceName}.${methodName}`,
+                0.85,
+                {
+                  package: info.package,
+                  service: info.serviceName,
+                  method: methodName,
+                  source: 'proto',
+                },
+              ),
+            );
+          }
         }
       }
     }
@@ -422,29 +429,6 @@ export class GrpcExtractor implements ContractExtractor {
     return this.dedupe(out);
   }
 
-  private parseProtoFile(content: string, filePath: string, pkg: string): ExtractedContract[] {
-    const out: ExtractedContract[] = [];
-
-    for (const { name: serviceName, body } of extractServiceBlocks(content)) {
-      const rpcRe = /rpc\s+(\w+)\s*\(/g;
-      let rpcMatch: RegExpExecArray | null;
-      while ((rpcMatch = rpcRe.exec(body)) !== null) {
-        const methodName = rpcMatch[1];
-        const cid = contractId(pkg, serviceName, methodName);
-        out.push(
-          makeContract(cid, 'provider', filePath, `${serviceName}.${methodName}`, 0.85, {
-            package: pkg,
-            service: serviceName,
-            method: methodName,
-            source: 'proto',
-          }),
-        );
-      }
-    }
-
-    return out;
-  }
-
   /**
    * Convert a plugin `GrpcDetection` into a concrete `ExtractedContract`
    * by resolving the short service name against the proto map, building

gitnexus/src/core/group/extractors/grpc-patterns/node.ts

@@ -82,12 +82,28 @@ const NEW_QUALIFIED_CTOR_SPEC: PatternSpec<Record<string, never>> = {
   `,
 };
 
+// Detect whether the file uses `loadPackageDefinition` (gRPC dynamic
+// proto loader). Matches either a bare call or an `obj.loadPackageDefinition(...)`
+// call. Plugin gates the qualified-constructor consumer on this —
+// structural check avoids materializing `tree.rootNode.text` for every file.
+const LOAD_PACKAGE_DEFINITION_SPEC: PatternSpec<Record<string, never>> = {
+  meta: {},
+  query: `
+    (call_expression
+      function: [
+        (identifier) @fn (#eq? @fn "loadPackageDefinition")
+        (member_expression property: (property_identifier) @fn (#eq? @fn "loadPackageDefinition"))
+      ])
+  `,
+};
+
 interface NodeGrpcPatternBundle {
   grpcMethod: CompiledPatterns<Record<string, never>>;
   grpcClient: CompiledPatterns<Record<string, never>>;
   getService: CompiledPatterns<Record<string, never>>;
   newSimpleCtor: CompiledPatterns<Record<string, never>>;
   newQualifiedCtor: CompiledPatterns<Record<string, never>>;
+  loadPackageDefinition: CompiledPatterns<Record<string, never>>;
 }
 
 function compileBundle(language: unknown, name: string): NodeGrpcPatternBundle {
@@ -103,6 +119,7 @@ function compileBundle(language: unknown, name: string): NodeGrpcPatternBundle {
     getService: mk(GET_SERVICE_SPEC, 'get-service'),
     newSimpleCtor: mk(NEW_SIMPLE_CTOR_SPEC, 'new-simple-ctor'),
     newQualifiedCtor: mk(NEW_QUALIFIED_CTOR_SPEC, 'new-qualified-ctor'),
+    loadPackageDefinition: mk(LOAD_PACKAGE_DEFINITION_SPEC, 'load-package-definition'),
   };
 }
 
@@ -255,8 +272,10 @@ function scanBundle(bundle: NodeGrpcPatternBundle, tree: Parser.Tree): GrpcDetec
   // ─── Consumer: loadPackageDefinition dynamic proto loader ────────
   // Only emit when the file uses loadPackageDefinition, otherwise a
   // generic `new foo.bar.Something()` in unrelated code would falsely
-  // register as a gRPC consumer.
-  const usesLoadPackage = tree.rootNode.text.includes('loadPackageDefinition');
+  // register as a gRPC consumer. Check structurally via a dedicated
+  // query — avoids materializing `tree.rootNode.text` for the whole
+  // file (expensive on large files).
+  const usesLoadPackage = runCompiledPatterns(bundle.loadPackageDefinition, tree).length > 0;
   if (usesLoadPackage) {
     for (const match of runCompiledPatterns(bundle.newQualifiedCtor, tree)) {
       const ctorNode = match.captures.ctor;

gitnexus/src/core/group/extractors/http-route-extractor.ts

@@ -63,7 +63,10 @@ export function normalizeHttpPath(p: string): string {
   s = s.replace(/:\w+/g, '{param}');
   s = s.replace(/\{[^}]+\}/g, '{param}');
   s = s.replace(/\[[^\]]+\]/g, '{param}');
-  return s;
+  // Preserve root: after stripping trailing slashes, the root "/"
+  // collapses to "" which would produce malformed contract ids like
+  // `http::GET::`. Restore a single slash for the root case.
+  return s === '' ? '/' : s;
 }
 
 /**
@@ -192,19 +195,28 @@ export class HttpRouteExtractor implements ContractExtractor {
       }
     };
 
+    // Glob the source-scan file list at most once per extract() —
+    // both provider and consumer fallback paths share the same list.
+    let scannedFiles: string[] | null = null;
+    const getScannedFiles = async (): Promise<string[]> => {
+      if (scannedFiles) return scannedFiles;
+      scannedFiles = await this.scanFiles(repoPath);
+      return scannedFiles;
+    };
+
     const graphProviders =
       dbExecutor != null ? await this.extractProvidersGraph(dbExecutor, getDetections) : [];
     const providers =
       graphProviders.length > 0
         ? graphProviders
-        : this.extractProvidersSourceScan(await this.scanFiles(repoPath), getDetections);
+        : this.extractProvidersSourceScan(await getScannedFiles(), getDetections);
 
     const graphConsumers =
       dbExecutor != null ? await this.extractConsumersGraph(dbExecutor, getDetections) : [];
     const consumers =
       graphConsumers.length > 0
         ? graphConsumers
-        : this.extractConsumersSourceScan(await this.scanFiles(repoPath), getDetections);
+        : this.extractConsumersSourceScan(await getScannedFiles(), getDetections);
 
     return [...providers, ...consumers];
   }
@@ -212,7 +224,7 @@ export class HttpRouteExtractor implements ContractExtractor {
   private async scanFiles(repoPath: string): Promise<string[]> {
     return glob(HTTP_SCAN_GLOB, {
       cwd: repoPath,
-      ignore: ['**/node_modules/**', '**/.git/**', '**/dist/**', '**/build/**'],
+      ignore: ['**/node_modules/**', '**/.git/**', '**/dist/**', '**/build/**', '**/vendor/**'],
       nodir: true,
     });
   }