From 04a4fcbea7aacdbf98d5a5e5e60611e533702bdf Mon Sep 17 00:00:00 2001
From: Gergo Magyar <gergomagyar@icloud.com>
Date: Tue, 23 Jun 2026 17:43:32 +0000
Subject: [PATCH] fix(review): apply autofix feedback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Code-review autofix pass on the #2279 branch:
- Replace a newly-introduced `mode as any` cast in the #2279 it.each with the
  narrow `mode as 'callgraph' | undefined` (strict-typing-no-any).
- Add a degradation test for the new logQueryError benign-missing-table → debug
  branch (asserts no warn/error record surfaces, i.e. it routed to debug).
- Pin the bm25/FTS error→warn severity change with a _captureLogger assertion
  in the existing #1489 test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 gitnexus/test/unit/calltool-dispatch.test.ts | 73 ++++++++++++++++++--
 1 file changed, 67 insertions(+), 6 deletions(-)

diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts
index e8fad5928..e65d9c061 100644
--- a/gitnexus/test/unit/calltool-dispatch.test.ts
+++ b/gitnexus/test/unit/calltool-dispatch.test.ts
@@ -395,12 +395,23 @@ describe('LocalBackend.callTool', () => {
     vi.mocked(searchFTSFromLbug).mockRejectedValueOnce(new Error('bm25Results is not iterable'));
     (executeParameterized as any).mockResolvedValue([]);
 
-    const result = await backend.callTool('query', { query: 'auth' });
+    const cap = _captureLogger();
+    try {
+      const result = await backend.callTool('query', { query: 'auth' });
 
-    // Should still return a valid result shape (semantic-only fallback)
-    expect(result).toHaveProperty('processes');
-    expect(result).toHaveProperty('definitions');
-    expect(result).not.toHaveProperty('error');
+      // Should still return a valid result shape (semantic-only fallback)
+      expect(result).toHaveProperty('processes');
+      expect(result).toHaveProperty('definitions');
+      expect(result).not.toHaveProperty('error');
+      // The FTS fallback is a gracefully-degraded result, not an operation failure:
+      // it must log at warn (40), never error (50), matching its sibling
+      // import-failure fallback. Pins the severity against regression.
+      const fts = cap.records().find((r) => /BM25\/FTS search failed/.test(String(r.msg ?? '')));
+      expect(fts).toBeDefined();
+      expect(fts?.level).toBe(40);
+    } finally {
+      cap.restore();
+    }
   });
 
   it('skips vector index query when VECTOR is unsupported by the platform', async () => {
@@ -1655,7 +1666,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => {
       const result = await backend.callTool('impact', {
         target: 'main',
         direction: 'upstream',
-        mode: mode as any,
+        mode: mode as 'callgraph' | undefined,
         line: 0,
       });
       // No PDG-only error, no positive-integer error — line:0 is swallowed.
@@ -1889,6 +1900,56 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => {
     }
   });
 
+  it("mode:'pdg' slice-callees failing with a benign missing-table error logs at debug, not warn", async () => {
+    // A repo analyzed without the optional column/table (e.g. a pre-v3 PDG index
+    // missing `calleeIds`, or a BasicBlock table that simply isn't there) makes the
+    // slice-callees query fail with a benign "missing optional data" error. That is a
+    // normal configuration, not a degradation, so logQueryError routes it to debug —
+    // suppressed at the default info level. The capture logger runs at info, so a
+    // debug record never reaches it: the assertion is that NO info+ record (warn/error)
+    // appears for this context, distinguishing the benign branch from the warn branch
+    // pinned by the test above.
+    resolveSingleTarget();
+    vi.mocked(executeParameterized).mockImplementation(async (_repo, query) => {
+      if (query.includes('RETURN b.callees')) throw new Error('Table BasicBlock does not exist');
+      return [{ id: 'func:main', name: 'main', type: 'Function', filePath: 'src/index.ts' }];
+    });
+    vi.spyOn(backend as any, '_runImpactPDG').mockResolvedValueOnce({
+      mode: 'pdg',
+      target: { id: 'func:main', name: 'main', type: 'Function', filePath: 'src/index.ts' },
+      direction: 'downstream',
+      risk: 'UNKNOWN',
+      impactedCount: 0,
+      epistemic: 'pdg-intra-procedural',
+      reachableBlocks: ['BasicBlock:src/index.ts:8:0:1'],
+      intraReachableBlocks: ['BasicBlock:src/index.ts:8:0:1'],
+      seedBlocks: ['BasicBlock:src/index.ts:8:0:0'],
+      blockCount: 1,
+      affectedStatements: [{ line: 8, filePath: 'src/index.ts', text: 'callee()' }],
+      affectedStatementCount: 1,
+      criterionLine: 8,
+    });
+    const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');
+    const cap = _captureLogger();
+    try {
+      const result = await backend.callTool('impact', {
+        target: 'main',
+        direction: 'downstream',
+        mode: 'pdg',
+        line: 8,
+      });
+      // Still degrades cleanly to no bridge / no surfaced error.
+      expect(result.error).toBeUndefined();
+      expect(bfsSpy.mock.calls[0][4].pdgBridge).toBeUndefined();
+      // The benign failure was routed to debug (suppressed at info): no warn/error
+      // record for this context. A regression to warn/error would surface here.
+      const slice = cap.records().find((r) => r.context === 'impact:pdg-slice-callees');
+      expect(slice).toBeUndefined();
+    } finally {
+      cap.restore();
+    }
+  });
+
   it("mode:'pdg' + crossDepth → hard {error} (single-repo PDG impact)", async () => {
     resolveSingleTarget();
     const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');