mirror of
https://github.com/giancarloerra/socraticode.git
synced 2026-07-03 14:05:21 +02:00
fix(graph): normalize Windows backslash paths to forward slashes
On Windows, path.relative() and path.join() return backslash separators. Graph node keys were stored with native separators, but query inputs use forward slashes, causing silent lookup failures on Windows. Add toForwardSlash() utility and apply it at build time (file walker, resolution functions) and query time (getFileDependencies, getSymbolContext, listSymbols) for defense-in-depth. No-op on macOS/Linux where path.relative() already returns forward slashes. Existing Windows symbol graph caches require one rebuild. Fixes #60
This commit is contained in:
@@ -165,6 +165,18 @@ export const ENTRY_POINT_NAMES: Record<string, Set<string>> = {
|
|||||||
php: new Set(["main"]),
|
php: new Set(["main"]),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// ── Path normalization ──────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Normalize path separators to forward slashes.
|
||||||
|
* On POSIX this is a no-op; on Windows it replaces every `\` with `/`.
|
||||||
|
* Used for graph node keys, fileSet entries, and query inputs so that
|
||||||
|
* lookups succeed regardless of the host OS separator convention.
|
||||||
|
*/
|
||||||
|
export function toForwardSlash(p: string): string {
|
||||||
|
return p.replace(/\\/g, "/");
|
||||||
|
}
|
||||||
|
|
||||||
// ── File type configuration ─────────────────────────────────────────────
|
// ── File type configuration ─────────────────────────────────────────────
|
||||||
|
|
||||||
export const SUPPORTED_EXTENSIONS = new Set([
|
export const SUPPORTED_EXTENSIONS = new Set([
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import { createRequire } from "node:module";
|
|||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { Lang, registerDynamicLanguage } from "@ast-grep/napi";
|
import { Lang, registerDynamicLanguage } from "@ast-grep/napi";
|
||||||
import { graphCollectionName, projectIdFromPath } from "../config.js";
|
import { graphCollectionName, projectIdFromPath } from "../config.js";
|
||||||
import { EXTRA_EXTENSIONS, getLanguageFromExtension, MAX_GRAPH_FILE_BYTES } from "../constants.js";
|
import { EXTRA_EXTENSIONS, getLanguageFromExtension, MAX_GRAPH_FILE_BYTES, toForwardSlash } from "../constants.js";
|
||||||
import type {
|
import type {
|
||||||
CodeGraph, CodeGraphEdge, CodeGraphNode,
|
CodeGraph, CodeGraphEdge, CodeGraphNode,
|
||||||
SymbolEdge, SymbolGraphFilePayload, SymbolGraphMeta, SymbolNode, SymbolRef,
|
SymbolEdge, SymbolGraphFilePayload, SymbolGraphMeta, SymbolNode, SymbolRef,
|
||||||
@@ -596,7 +596,7 @@ async function getGraphableFiles(
|
|||||||
|
|
||||||
for (const entry of entries) {
|
for (const entry of entries) {
|
||||||
const fullPath = path.join(dir, entry.name);
|
const fullPath = path.join(dir, entry.name);
|
||||||
const relPath = path.relative(projectPath, fullPath);
|
const relPath = toForwardSlash(path.relative(projectPath, fullPath));
|
||||||
|
|
||||||
if (shouldIgnore(ig, relPath)) continue;
|
if (shouldIgnore(ig, relPath)) continue;
|
||||||
|
|
||||||
|
|||||||
@@ -1,17 +1,20 @@
|
|||||||
// SPDX-License-Identifier: AGPL-3.0-only
|
// SPDX-License-Identifier: AGPL-3.0-only
|
||||||
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { getLanguageFromExtension } from "../constants.js";
|
import { getLanguageFromExtension, toForwardSlash } from "../constants.js";
|
||||||
import type { CodeGraph } from "../types.js";
|
import type { CodeGraph } from "../types.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get dependencies for a specific file.
|
* Get dependencies for a specific file.
|
||||||
|
* The input path is normalized to forward slashes so lookups succeed
|
||||||
|
* regardless of whether the caller passes `/` or `\` separators.
|
||||||
*/
|
*/
|
||||||
export function getFileDependencies(graph: CodeGraph, relativePath: string): {
|
export function getFileDependencies(graph: CodeGraph, relativePath: string): {
|
||||||
imports: string[];
|
imports: string[];
|
||||||
importedBy: string[];
|
importedBy: string[];
|
||||||
} {
|
} {
|
||||||
const node = graph.nodes.find((n) => n.relativePath === relativePath);
|
const normalized = toForwardSlash(relativePath);
|
||||||
|
const node = graph.nodes.find((n) => n.relativePath === normalized);
|
||||||
if (!node) {
|
if (!node) {
|
||||||
return { imports: [], importedBy: [] };
|
return { imports: [], importedBy: [] };
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,7 +7,7 @@
|
|||||||
* lazy-loaded per-file payloads.
|
* lazy-loaded per-file payloads.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { MAX_FLOW_DEPTH, MAX_IMPACT_DEPTH } from "../constants.js";
|
import { MAX_FLOW_DEPTH, MAX_IMPACT_DEPTH, toForwardSlash } from "../constants.js";
|
||||||
import type { SymbolNode } from "../types.js";
|
import type { SymbolNode } from "../types.js";
|
||||||
import {
|
import {
|
||||||
type SymbolGraphCache,
|
type SymbolGraphCache,
|
||||||
@@ -184,7 +184,10 @@ export async function getSymbolContext(
|
|||||||
): Promise<SymbolContext[]> {
|
): Promise<SymbolContext[]> {
|
||||||
const nameIndex = await cache.getNameIndex();
|
const nameIndex = await cache.getNameIndex();
|
||||||
let refs = nameIndex.get(name) ?? [];
|
let refs = nameIndex.get(name) ?? [];
|
||||||
if (fileHint) refs = refs.filter((r) => r.file === fileHint);
|
if (fileHint) {
|
||||||
|
const normalizedHint = toForwardSlash(fileHint);
|
||||||
|
refs = refs.filter((r) => r.file === normalizedHint);
|
||||||
|
}
|
||||||
if (refs.length === 0) return [];
|
if (refs.length === 0) return [];
|
||||||
|
|
||||||
const reverseIndex = await cache.getReverseFileIndex();
|
const reverseIndex = await cache.getReverseFileIndex();
|
||||||
@@ -237,7 +240,7 @@ export async function listSymbols(
|
|||||||
const out: SymbolNode[] = [];
|
const out: SymbolNode[] = [];
|
||||||
|
|
||||||
if (opts.file) {
|
if (opts.file) {
|
||||||
const payload = await cache.getFilePayload(opts.file);
|
const payload = await cache.getFilePayload(toForwardSlash(opts.file));
|
||||||
if (!payload) return [];
|
if (!payload) return [];
|
||||||
for (const s of payload.symbols) {
|
for (const s of payload.symbols) {
|
||||||
if (s.name === "<module>") continue;
|
if (s.name === "<module>") continue;
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
||||||
import { readFileSync } from "node:fs";
|
import { readFileSync } from "node:fs";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
|
import { toForwardSlash } from "../constants.js";
|
||||||
import type { PathAliases } from "./graph-aliases.js";
|
import type { PathAliases } from "./graph-aliases.js";
|
||||||
|
|
||||||
// ── Module resolution ────────────────────────────────────────────────────
|
// ── Module resolution ────────────────────────────────────────────────────
|
||||||
@@ -12,7 +13,7 @@ import type { PathAliases } from "./graph-aliases.js";
|
|||||||
* For a Maven/Gradle multi-module layout such as:
|
* For a Maven/Gradle multi-module layout such as:
|
||||||
* module-a/sub-module/src/main/java/com/example/Foo.java
|
* module-a/sub-module/src/main/java/com/example/Foo.java
|
||||||
* the map entry is:
|
* the map entry is:
|
||||||
* key: "com/example/Foo.java" (platform-normalised with path.sep)
|
* key: "com/example/Foo.java" (forward-slash-normalized)
|
||||||
* value: "module-a/sub-module/src/main/java/com/example/Foo.java"
|
* value: "module-a/sub-module/src/main/java/com/example/Foo.java"
|
||||||
*
|
*
|
||||||
* This enables O(1) resolution of fully-qualified class names that cannot be
|
* This enables O(1) resolution of fully-qualified class names that cannot be
|
||||||
@@ -41,7 +42,7 @@ export function buildJvmSuffixMap(fileSet: Set<string>): Map<string, string> {
|
|||||||
|
|
||||||
if (idx !== -1) {
|
if (idx !== -1) {
|
||||||
// classPath = everything after src/main/<lang>, e.g. com/example/Foo.java
|
// classPath = everything after src/main/<lang>, e.g. com/example/Foo.java
|
||||||
const classPath = parts.slice(idx + 3).join(path.sep);
|
const classPath = parts.slice(idx + 3).join("/");
|
||||||
// Only register the first match to avoid ambiguity for duplicate class names.
|
// Only register the first match to avoid ambiguity for duplicate class names.
|
||||||
if (!map.has(classPath)) {
|
if (!map.has(classPath)) {
|
||||||
map.set(classPath, f);
|
map.set(classPath, f);
|
||||||
@@ -184,13 +185,9 @@ export function buildGoModuleInfo(
|
|||||||
|
|
||||||
const packageMap = new Map<string, string>();
|
const packageMap = new Map<string, string>();
|
||||||
for (const f of goFiles) {
|
for (const f of goFiles) {
|
||||||
// Normalize the directory key to forward slashes. Go import paths
|
// Go import paths always use forward slashes. fileSet entries are
|
||||||
// always use forward slashes regardless of host OS, so the key must
|
// also forward-slash-normalized (see toForwardSlash in constants.ts),
|
||||||
// be in the same form for the lookup in resolveImport to succeed on
|
// so the key and value are both in the same form.
|
||||||
// Windows (where path.dirname produces backslash separators for
|
|
||||||
// nested directories like `pkg\subpkg`). The map value keeps the
|
|
||||||
// file's native-separator form so it matches fileSet entries used
|
|
||||||
// elsewhere as graph node keys.
|
|
||||||
const dir = path.dirname(f).replace(/\\/g, "/"); // "." for files at the project root
|
const dir = path.dirname(f).replace(/\\/g, "/"); // "." for files at the project root
|
||||||
if (!packageMap.has(dir)) {
|
if (!packageMap.has(dir)) {
|
||||||
packageMap.set(dir, f);
|
packageMap.set(dir, f);
|
||||||
@@ -351,7 +348,7 @@ export function resolveImport(
|
|||||||
// The map is built once per graph build (O(n)) and looked up in O(1).
|
// The map is built once per graph build (O(n)) and looked up in O(1).
|
||||||
if (jvmSuffixMap) {
|
if (jvmSuffixMap) {
|
||||||
for (const ext of exts) {
|
for (const ext of exts) {
|
||||||
const classPath = filePath.replace(/\//g, path.sep) + ext;
|
const classPath = filePath + ext;
|
||||||
const found = jvmSuffixMap.get(classPath);
|
const found = jvmSuffixMap.get(classPath);
|
||||||
if (found) return found;
|
if (found) return found;
|
||||||
}
|
}
|
||||||
@@ -419,7 +416,7 @@ export function resolveImport(
|
|||||||
path.join(sourceDir, moduleSpecifier, "mod.rs"),
|
path.join(sourceDir, moduleSpecifier, "mod.rs"),
|
||||||
];
|
];
|
||||||
for (const candidate of candidates) {
|
for (const candidate of candidates) {
|
||||||
const rel = path.relative(projectPath, candidate);
|
const rel = toForwardSlash(path.relative(projectPath, candidate));
|
||||||
if (fileSet.has(rel)) return rel;
|
if (fileSet.has(rel)) return rel;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -564,7 +561,7 @@ function resolveRelativePath(
|
|||||||
extensions: string[],
|
extensions: string[],
|
||||||
): string | null {
|
): string | null {
|
||||||
const fullPath = path.resolve(baseDir, modulePath);
|
const fullPath = path.resolve(baseDir, modulePath);
|
||||||
const relPath = path.relative(projectPath, fullPath);
|
const relPath = toForwardSlash(path.relative(projectPath, fullPath));
|
||||||
|
|
||||||
// Direct match
|
// Direct match
|
||||||
if (fileSet.has(relPath)) return relPath;
|
if (fileSet.has(relPath)) return relPath;
|
||||||
@@ -590,7 +587,7 @@ function resolveRelativePath(
|
|||||||
|
|
||||||
// Try as directory with index file
|
// Try as directory with index file
|
||||||
for (const ext of extensions) {
|
for (const ext of extensions) {
|
||||||
const indexFile = path.join(relPath, `index${ext}`);
|
const indexFile = toForwardSlash(path.join(relPath, `index${ext}`));
|
||||||
if (fileSet.has(indexFile)) return indexFile;
|
if (fileSet.has(indexFile)) return indexFile;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -600,11 +597,11 @@ function resolveRelativePath(
|
|||||||
const base = path.basename(relPath);
|
const base = path.basename(relPath);
|
||||||
if (!base.startsWith("_")) {
|
if (!base.startsWith("_")) {
|
||||||
// Try _name (direct)
|
// Try _name (direct)
|
||||||
const partial = path.join(dir, `_${base}`);
|
const partial = toForwardSlash(path.join(dir, `_${base}`));
|
||||||
if (fileSet.has(partial)) return partial;
|
if (fileSet.has(partial)) return partial;
|
||||||
// Try _name with extensions
|
// Try _name with extensions
|
||||||
for (const ext of extensions) {
|
for (const ext of extensions) {
|
||||||
const partialExt = path.join(dir, `_${base}${ext}`);
|
const partialExt = toForwardSlash(path.join(dir, `_${base}${ext}`));
|
||||||
if (fileSet.has(partialExt)) return partialExt;
|
if (fileSet.has(partialExt)) return partialExt;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -612,7 +609,7 @@ function resolveRelativePath(
|
|||||||
|
|
||||||
// Python: try __init__.py
|
// Python: try __init__.py
|
||||||
if (extensions.includes(".py")) {
|
if (extensions.includes(".py")) {
|
||||||
const initFile = path.join(relPath, "__init__.py");
|
const initFile = toForwardSlash(path.join(relPath, "__init__.py"));
|
||||||
if (fileSet.has(initFile)) return initFile;
|
if (fileSet.has(initFile)) return initFile;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import {
|
|||||||
SEARCH_MIN_SCORE,
|
SEARCH_MIN_SCORE,
|
||||||
SPECIAL_FILES,
|
SPECIAL_FILES,
|
||||||
SUPPORTED_EXTENSIONS,
|
SUPPORTED_EXTENSIONS,
|
||||||
|
toForwardSlash,
|
||||||
} from "../../src/constants.js";
|
} from "../../src/constants.js";
|
||||||
|
|
||||||
describe("constants", () => {
|
describe("constants", () => {
|
||||||
@@ -373,6 +374,37 @@ describe("constants", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("toForwardSlash", () => {
|
||||||
|
it("returns the same string when no backslashes are present", () => {
|
||||||
|
expect(toForwardSlash("src/index.ts")).toBe("src/index.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("replaces single backslash with forward slash", () => {
|
||||||
|
expect(toForwardSlash("src\\index.ts")).toBe("src/index.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("replaces multiple backslashes in a path", () => {
|
||||||
|
expect(toForwardSlash("src\\services\\graph-analysis.ts")).toBe("src/services/graph-analysis.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("handles deeply nested Windows paths", () => {
|
||||||
|
expect(toForwardSlash("src\\a\\b\\c\\d\\file.ts")).toBe("src/a/b/c/d/file.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("handles empty string", () => {
|
||||||
|
expect(toForwardSlash("")).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("handles path with mixed separators", () => {
|
||||||
|
expect(toForwardSlash("src/services\\graph-analysis.ts")).toBe("src/services/graph-analysis.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is a no-op on POSIX-style paths", () => {
|
||||||
|
const posixPath = "src/services/code-graph.ts";
|
||||||
|
expect(toForwardSlash(posixPath)).toBe(posixPath);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("resolveQdrantPort", () => {
|
describe("resolveQdrantPort", () => {
|
||||||
it("returns explicit port from URL", () => {
|
it("returns explicit port from URL", () => {
|
||||||
expect(resolveQdrantPort("https://qdrant.example.com:6333")).toBe(6333);
|
expect(resolveQdrantPort("https://qdrant.example.com:6333")).toBe(6333);
|
||||||
|
|||||||
@@ -113,6 +113,38 @@ describe("graph-analysis", () => {
|
|||||||
expect(deps.imports).toHaveLength(0);
|
expect(deps.imports).toHaveLength(0);
|
||||||
expect(deps.importedBy).toHaveLength(0);
|
expect(deps.importedBy).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("normalizes Windows backslash paths to match forward-slash graph keys", () => {
|
||||||
|
const graph = createSampleGraph();
|
||||||
|
// Graph keys use forward slashes; simulate a Windows-style query
|
||||||
|
const deps = getFileDependencies(graph, "src\\index.ts");
|
||||||
|
|
||||||
|
expect(deps.imports).toContain("src/utils.ts");
|
||||||
|
expect(deps.imports).toContain("src/types.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes deeply nested Windows paths", () => {
|
||||||
|
const nodes: CodeGraphNode[] = [
|
||||||
|
makeNode("src/services/graph/analysis.ts", ["src/types.ts"], []),
|
||||||
|
makeNode("src/types.ts", [], ["src/services/graph/analysis.ts"]),
|
||||||
|
];
|
||||||
|
const edges: CodeGraphEdge[] = [
|
||||||
|
makeEdge("src/services/graph/analysis.ts", "src/types.ts"),
|
||||||
|
];
|
||||||
|
const graph = makeGraph(nodes, edges);
|
||||||
|
|
||||||
|
const deps = getFileDependencies(graph, "src\\services\\graph\\analysis.ts");
|
||||||
|
expect(deps.imports).toContain("src/types.ts");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("handles mixed separator paths", () => {
|
||||||
|
const graph = createSampleGraph();
|
||||||
|
const deps = getFileDependencies(graph, "src/utils.ts");
|
||||||
|
const depsMixed = getFileDependencies(graph, "src\\utils.ts");
|
||||||
|
|
||||||
|
expect(depsMixed.imports).toEqual(deps.imports);
|
||||||
|
expect(depsMixed.importedBy).toEqual(deps.importedBy);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("findCircularDependencies", () => {
|
describe("findCircularDependencies", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user