mirror of
https://github.com/giancarloerra/socraticode.git
synced 2026-06-02 06:23:43 +02:00
fix: address CodeRabbit review findings
This commit is contained in:
@@ -33,4 +33,5 @@ coverage/
|
||||
|
||||
# Snyk Security Extension - AI Rules (auto-generated)
|
||||
# Personal instructions (Copilot)
|
||||
.github/copilot-instructions.md
|
||||
.github/instructions/
|
||||
|
||||
@@ -55,7 +55,6 @@ before reading any files directly.
|
||||
| Verify index is up to date | `codebase_status` |
|
||||
| Discover what project knowledge (schemas, specs, configs) is available | `codebase_context` |
|
||||
| Find database tables, API endpoints, infra configs | `codebase_context_search` |
|
||||
```
|
||||
|
||||
> **Why semantic search first?** A single `codebase_search` call returns ranked, deduplicated snippets from across the entire codebase in milliseconds. This gives you a broad map at negligible token cost — far cheaper than opening files speculatively. Once you know which files matter, targeted reading is both faster and more accurate. That said, grep remains the right tool when you have an exact string or pattern — use whichever fits the query.
|
||||
|
||||
|
||||
@@ -115,8 +115,3 @@ This project uses [Conventional Commits](https://www.conventionalcommits.org/) a
|
||||
### Bug Fixes
|
||||
|
||||
* add mcpName and read version dynamically from package.json ([88c0e8f](https://github.com/giancarloerra/socraticode/commit/88c0e8fee39c7fb733bdec4657d2eaf2c355292e))
|
||||
|
||||
# Changelog
|
||||
|
||||
All notable changes to SocratiCode are documented here.
|
||||
This project uses [Conventional Commits](https://www.conventionalcommits.org/) and [Semantic Versioning](https://semver.org/).
|
||||
|
||||
@@ -55,7 +55,6 @@ before reading any files directly.
|
||||
| Verify index is up to date | `codebase_status` |
|
||||
| Discover what project knowledge (schemas, specs, configs) is available | `codebase_context` |
|
||||
| Find database tables, API endpoints, infra configs | `codebase_context_search` |
|
||||
```
|
||||
|
||||
> **Why semantic search first?** A single `codebase_search` call returns ranked, deduplicated snippets from across the entire codebase in milliseconds. This gives you a broad map at negligible token cost — far cheaper than opening files speculatively. Once you know which files matter, targeted reading is both faster and more accurate. That said, grep remains the right tool when you have an exact string or pattern — use whichever fits the query.
|
||||
|
||||
|
||||
+2
-2
@@ -211,7 +211,7 @@ tests/
|
||||
├── helpers/
|
||||
│ ├── fixtures.ts # Test fixture utilities (temp projects, Docker checks)
|
||||
│ └── setup.ts # Integration test infrastructure (Qdrant client, cleanup)
|
||||
├── unit/ # 460 tests — no Docker required
|
||||
├── unit/ # 608 tests — no Docker required
|
||||
├── integration/ # 137 tests — requires Docker
|
||||
└── e2e/ # 20 tests — full lifecycle
|
||||
|
||||
@@ -543,7 +543,7 @@ npm run test:coverage
|
||||
|
||||
| Layer | Directory | Tests | Docker | Description |
|
||||
|-------|-----------|-------|--------|-------------|
|
||||
| Unit | `tests/unit/` | 477 | No | Pure logic: config, constants, ignore rules, cross-process locking, logging, graph analysis, import extraction, path resolution, startup lifecycle |
|
||||
| Unit | `tests/unit/` | 608 | No | Pure logic: config, constants, ignore rules, cross-process locking, logging, graph analysis, import extraction, path resolution, startup lifecycle |
|
||||
| Integration | `tests/integration/` | 137 | Yes | Real Docker containers: Qdrant CRUD, real Ollama embeddings, indexer, watcher, code graph, all 21 MCP tools |
|
||||
| E2E | `tests/e2e/` | 20 | Yes | Full lifecycle: health check → index → search → graph build/query/stats → watch → remove |
|
||||
|
||||
|
||||
@@ -74,7 +74,7 @@ The first Qdrant‑based MCP/Claude Plugin/Skill that pairs auto‑managed, zero
|
||||
|
||||
> **Only [Docker](https://www.docker.com/products/docker-desktop/) (running) required.**
|
||||
|
||||
**One-click install** — Claude Code, VS Code and Cursor,:
|
||||
**One-click install** — Claude Code, VS Code and Cursor:
|
||||
|
||||
[](#claude-code-plugin-recommended-for-claude-code-users)
|
||||
[](https://insiders.vscode.dev/redirect/mcp/install?name=socraticode&config=%7B%22command%22%3A%22npx%22%2C%22args%22%3A%5B%22-y%22%2C%22socraticode%22%5D%7D) [](https://insiders.vscode.dev/redirect/mcp/install?name=socraticode&config=%7B%22command%22%3A%22npx%22%2C%22args%22%3A%5B%22-y%22%2C%22socraticode%22%5D%7D&quality=insiders) [](cursor://anysphere.cursor-deeplink/mcp/install?name=socraticode&config=eyJjb21tYW5kIjoibnB4IiwiYXJncyI6WyIteSIsInNvY3JhdGljb2RlIl19)
|
||||
|
||||
+1
-1
@@ -6,7 +6,7 @@
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "echo 'IMPORTANT: If you notice that SocratiCode MCP tools appear with BOTH a plugin prefix (e.g. mcp__plugin_socraticode_socraticode__codebase_search) AND a standalone prefix (e.g. mcp__socraticode__codebase_search), the user has a duplicate configuration. Advise them to remove the standalone MCP by running: claude mcp remove socraticode — the plugin already provides the MCP server.'",
|
||||
"command": "echo 'IMPORTANT: If you notice that SocratiCode MCP tools appear with BOTH a plugin prefix (e.g. mcp__plugin_socraticode_socraticode__codebase_search) AND a standalone prefix (e.g. mcp__socraticode__codebase_search), the user has a duplicate configuration. Advise them to remove the standalone MCP by running: claude mcp remove socraticode - the plugin already provides the MCP server.'",
|
||||
"timeout": 5
|
||||
}
|
||||
]
|
||||
|
||||
@@ -129,10 +129,16 @@ export function loadEmbeddingConfig(): EmbeddingConfig {
|
||||
|
||||
// ── Model & dimensions (provider-specific defaults) ─────────────────
|
||||
const embeddingModel = process.env.EMBEDDING_MODEL || providerDefaults.model;
|
||||
const embeddingDimensions = parseInt(
|
||||
const rawDimensions = parseInt(
|
||||
process.env.EMBEDDING_DIMENSIONS || String(providerDefaults.dimensions),
|
||||
10,
|
||||
);
|
||||
if (Number.isNaN(rawDimensions)) {
|
||||
throw new Error(
|
||||
`Invalid EMBEDDING_DIMENSIONS: "${process.env.EMBEDDING_DIMENSIONS}". Must be a number.`,
|
||||
);
|
||||
}
|
||||
const embeddingDimensions = rawDimensions;
|
||||
|
||||
const contextLengthEnv = process.env.EMBEDDING_CONTEXT_LENGTH;
|
||||
|
||||
@@ -143,7 +149,15 @@ export function loadEmbeddingConfig(): EmbeddingConfig {
|
||||
embeddingModel,
|
||||
embeddingDimensions,
|
||||
embeddingContextLength: contextLengthEnv
|
||||
? parseInt(contextLengthEnv, 10)
|
||||
? (() => {
|
||||
const parsed = parseInt(contextLengthEnv, 10);
|
||||
if (Number.isNaN(parsed)) {
|
||||
throw new Error(
|
||||
`Invalid EMBEDDING_CONTEXT_LENGTH: "${contextLengthEnv}". Must be a number.`,
|
||||
);
|
||||
}
|
||||
return parsed;
|
||||
})()
|
||||
: guessContextLength(embeddingModel),
|
||||
ollamaApiKey: process.env.OLLAMA_API_KEY || undefined,
|
||||
};
|
||||
|
||||
@@ -250,7 +250,7 @@ export function extractImports(source: string, lang: Lang | string, _ext: string
|
||||
if (reqMatch) {
|
||||
imports.push({
|
||||
moduleSpecifier: reqMatch[1],
|
||||
isDynamic: text.includes("require_relative"),
|
||||
isDynamic: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -127,7 +127,7 @@ function findNestedGitignores(rootPath: string, currentPath: string, ig: Ignore)
|
||||
|
||||
if (fs.existsSync(gitignorePath)) {
|
||||
const content = fs.readFileSync(gitignorePath, "utf-8");
|
||||
const relDir = path.relative(rootPath, dirPath);
|
||||
const relDir = path.relative(rootPath, dirPath).split(path.sep).join("/");
|
||||
|
||||
// Prefix each pattern with the relative directory
|
||||
const lines = content.split("\n");
|
||||
|
||||
@@ -25,7 +25,10 @@ const LOG_LEVELS: Record<LogLevel, number> = {
|
||||
error: 3,
|
||||
};
|
||||
|
||||
const currentLevel: LogLevel = (process.env.SOCRATICODE_LOG_LEVEL as LogLevel) || "info";
|
||||
const envLevel = process.env.SOCRATICODE_LOG_LEVEL?.toLowerCase();
|
||||
const currentLevel: LogLevel = envLevel && envLevel in LOG_LEVELS
|
||||
? (envLevel as LogLevel)
|
||||
: "info";
|
||||
const logFilePath: string | undefined = process.env.SOCRATICODE_LOG_FILE || undefined;
|
||||
|
||||
// Write a separator so you can tell where each server session begins
|
||||
|
||||
@@ -117,6 +117,9 @@ export class GoogleEmbeddingProvider implements EmbeddingProvider {
|
||||
|
||||
async embedSingle(text: string): Promise<number[]> {
|
||||
const results = await this.embed([text]);
|
||||
if (results.length === 0) {
|
||||
throw new Error("Embedding failed: no result returned");
|
||||
}
|
||||
return results[0];
|
||||
}
|
||||
|
||||
|
||||
@@ -117,6 +117,9 @@ export class OpenAIEmbeddingProvider implements EmbeddingProvider {
|
||||
|
||||
async embedSingle(text: string): Promise<number[]> {
|
||||
const results = await this.embed([text]);
|
||||
if (results.length === 0) {
|
||||
throw new Error("Embedding failed: no result returned");
|
||||
}
|
||||
return results[0];
|
||||
}
|
||||
|
||||
@@ -151,10 +154,11 @@ export class OpenAIEmbeddingProvider implements EmbeddingProvider {
|
||||
model: string,
|
||||
dimensions: number,
|
||||
): Promise<number[][]> {
|
||||
const supportsDimensions = model.startsWith("text-embedding-3");
|
||||
const response = await client.embeddings.create({
|
||||
model,
|
||||
input: texts,
|
||||
dimensions,
|
||||
...(supportsDimensions ? { dimensions } : {}),
|
||||
});
|
||||
|
||||
// OpenAI returns embeddings sorted by index, but we ensure order
|
||||
|
||||
@@ -112,7 +112,7 @@ describe("code-graph service", () => {
|
||||
);
|
||||
if (helpersNode) {
|
||||
// If the graph correctly resolves imports, helpers should have dependents
|
||||
expect(helpersNode.dependents.length).toBeGreaterThanOrEqual(0);
|
||||
expect(helpersNode.dependents.length).toBeGreaterThan(0);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -304,6 +304,6 @@ describe("codebase_search — includeLinked parameter", () => {
|
||||
|
||||
// Should have the file but no project tag brackets
|
||||
expect(output).toContain("src/index.ts");
|
||||
expect(output).not.toMatch(/\[.*-.*\]/); // no [project-name] tag
|
||||
expect(output).not.toMatch(/\[[^\]]+\]\s*src\//); // no [project-name] tag before file path
|
||||
});
|
||||
});
|
||||
|
||||
@@ -279,6 +279,7 @@ describe("watcher (unit)", () => {
|
||||
|
||||
// package-lock.json has .json extension which IS in SUPPORTED_EXTENSIONS,
|
||||
// so the update should still trigger for that event
|
||||
expect(mockUpdateProjectIndex).toHaveBeenCalledTimes(1);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user