Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 68 additions & 3 deletions src/indexer/commands/search.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { isAbsolute, relative } from "node:path";
import { toToon } from "@app/json/lib/toon";
import { isInteractive } from "@app/utils/cli";
import { SafeJSON } from "@app/utils/json";
import type { SearchResult } from "@app/utils/search/types";
import { truncateText } from "@app/utils/string";
import * as p from "@clack/prompts";
import type { Command } from "commander";
import pc from "picocolors";
Comment thread
coderabbitai[bot] marked this conversation as resolved.
import { normalizeConfidence } from "../lib/confidence";
import { formatChunkDisplayName } from "../lib/display-name";
import { parseQueryWords } from "../lib/highlight";
Expand Down Expand Up @@ -56,7 +59,7 @@ export function registerSearchCommand(program: Command): void {
.command("search")
.description("Search across indexes")
.argument("<query>", "Search query")
.option("-i, --index <name>", "Search specific index (default: all)")
.option("-i, --index <name>", "Index to search (auto-detect from cwd; 'all' for all)")
.option("-m, --mode <mode>", "Search mode: fulltext, vector, hybrid (default: auto-detect)")
.option("-l, --limit <n>", "Max results", parseInt, 20)
.option("-f, --file <filter>", "Filter results to files matching this substring")
Expand All @@ -82,14 +85,67 @@ export function registerSearchCommand(program: Command): void {
return;
}

const names = opts.index ? [opts.index] : manager.getIndexNames();
let names: string[];

if (opts.index === "all") {
names = manager.getIndexNames();
} else if (opts.index) {
names = [opts.index];
} else {
Comment on lines +90 to +94

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

all is now a silent reserved index name.

Index names are unconstrained strings, so a real index named all can no longer be targeted: --index all always fans out to every index. Either reserve/disallow that name when indexes are created, or move the fan-out behavior to a separate flag.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/indexer/commands/search.ts` around lines 90 - 94, The search command
currently treats the literal index name "all" as a fan-out, preventing targeting
a real index named "all"; instead introduce an explicit boolean flag (e.g.,
opts.all or --all) for fan-out and keep opts.index as a free string. Change the
logic in the search handler to: if (opts.all) names = manager.getIndexNames();
else if (opts.index) names = [opts.index]; and add the new CLI option in the
command option parsing (where the search command defines flags), updating help
text and any tests to use --all for global search so real indexes named "all"
remain valid.

const allMeta = manager.listIndexes();

if (allMeta.length === 0) {
p.log.info("No indexes configured. Run: tools indexer add <path>");
return;
}

if (allMeta.length === 1) {
names = [allMeta[0].name];
} else {
const cwd = process.cwd();
const cwdMatches = allMeta
.filter((m) => {
const rel = relative(m.config.baseDir, cwd);
return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel));
})
.sort((a, b) => b.config.baseDir.length - a.config.baseDir.length);

const cwdMatch = cwdMatches[0];
const isUniqueDeepest =
cwdMatches.length < 2 ||
cwdMatches[0].config.baseDir.length > cwdMatches[1].config.baseDir.length;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (cwdMatch && isUniqueDeepest) {
names = [cwdMatch.name];
p.log.info(pc.dim(`(using index "${cwdMatch.name}" from cwd)`));
} else if (isInteractive()) {
Comment on lines +105 to +121

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use the deepest path match for cwd auto-detection.

This find() is order-dependent and POSIX-only: a parent index like /repo can shadow /repo/app, sibling prefixes like /repo2 can match /repo, and \ paths on Windows never satisfy ${baseDir}/. Filter all matching indexes with path.relative() and either pick the deepest unique match or fall back to the existing prompt/error path.

🛠️ Suggested fix
+import { isAbsolute, relative, sep } from "node:path";
@@
-                        const cwdMatch = allMeta.find((m) => {
-                            const baseDir = m.config.baseDir;
-                            return cwd === baseDir || cwd.startsWith(`${baseDir}/`);
-                        });
+                        const cwdMatches = allMeta
+                            .filter((m) => {
+                                const rel = relative(m.config.baseDir, cwd);
+                                return rel === "" || (rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel));
+                            })
+                            .sort((a, b) => b.config.baseDir.length - a.config.baseDir.length);
+
+                        const cwdMatch = cwdMatches[0];
+                        const isUniqueDeepest =
+                            cwdMatches.length < 2 || cwdMatches[0].config.baseDir.length > cwdMatches[1].config.baseDir.length;
 
-                        if (cwdMatch) {
+                        if (cwdMatch && isUniqueDeepest) {
                             names = [cwdMatch.name];
                             p.log.info(pc.dim(`(using index "${cwdMatch.name}" from cwd)`));
                         } else if (isInteractive()) {

As per coding guidelines, use Node.js path module for cross-platform path handling, resolve relative paths to absolute using resolve(), and check file/directory existence before operations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/indexer/commands/search.ts` around lines 104 - 113, The current cwd
auto-detection uses an order-dependent, POSIX-only find over allMeta which can
wrongly match parents or siblings and fails on Windows; modify the logic that
computes cwdMatch (and the block that sets names and logs the choice) to:
resolve cwd and each m.config.baseDir with path.resolve, compute path.relative
from baseDir to cwd and accept matches where the relative path does not start
with '..' and is not an absolute traversal, collect all matches, pick the
deepest unique match by choosing the one with the longest resolved baseDir (or
most path segments), and if none found fall back to the existing
isInteractive()/prompt/error behavior; also ensure you check existence of the
resolved baseDir (fs.existsSync or fs.stat) before treating it as a match and
keep using the same names and logging behavior when a match is selected.

const allNames = allMeta.map((m) => m.name);
const selected = await p.select({
message: "Select index to search",
options: [
{ value: "__all__", label: "All indexes" },
...allNames.map((n) => ({ value: n, label: n })),
],
});

if (p.isCancel(selected)) {
p.log.info("Cancelled");
return;
}

names = selected === "__all__" ? allNames : [selected];
} else {
p.log.error("Multiple indexes found. Use -i <name> or -i all.");
return;
Comment on lines +95 to +139

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract this selection flow into src/indexer/lib/.

This branch now owns cwd matching, deepest-match disambiguation, prompt fallback, and selection policy. Moving it out of the Commander action will keep the command wrapper thin and make the behavior easier to unit-test.

As per coding guidelines, "Treat src/<tool>/commands/ files as thin wrappers — parse args, call into src/<tool>/lib/ for business logic. Keep commands lean, keep logic in lib/"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/indexer/commands/search.ts` around lines 95 - 139, Extract the
index-selection flow (the logic around manager.listIndexes(), cwd matching using
relative(..., cwd), deepest-match disambiguation, isInteractive() prompt
fallback with p.select and p.isCancel, and the resulting assignment to the names
variable) out of the Commander action and into a new function in
src/indexer/lib/ (e.g., selectIndex or resolveIndexSelection). Implement the new
function to accept the manager and cwd (and an interactive prompt helper or p
wrapper) and return the selected names array or throw/return a sentinel on
cancel; then replace the inlined block in search.ts with a thin call to that
function and handle its return (logging the same messages like the pc.dim info,
"Cancelled", and the error for multiple indexes) so behavior remains identical
but testable in the lib module.

}
}
}

if (names.length === 0) {
p.log.info("No indexes configured. Run: tools indexer add <path>");
return;
}

// Load all indexers upfront for mode detection across all indexes
const indexers = await Promise.all(names.map((n) => manager.getIndex(n)));

let mode: SearchMode;
Expand Down Expand Up @@ -171,13 +227,22 @@ export function registerSearchCommand(program: Command): void {
return;
}

const baseDirs = new Map<string, string>();

for (const indexer of indexers) {
const cfg = indexer.getConfig();
baseDirs.set(cfg.name, cfg.baseDir);
}

const words = parseQueryWords(query);
const output = formatSearchResults({
results: filtered,
format: format as OutputFormat,
query,
mode: effectiveMode,
highlightWords: words,
baseDirs,
multiIndex: names.length > 1,
});
console.log(output);
} finally {
Expand Down
100 changes: 100 additions & 0 deletions src/indexer/lib/chunker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1092,4 +1092,104 @@ import { c } from "./c";
expect(result.chunks.length).toBe(1);
});
});

describe("Heading strategy — improved splitting", () => {
it("preserves heading context in sub-chunks of large sections", () => {
const bigBody = Array.from({ length: 200 }, (_, i) => `Line ${i} of content with enough words to consume tokens.`).join("\n\n");
const content = `## Big Section\n\n${bigBody}`;

const result = chunkFile({
filePath: "doc.md",
content,
strategy: "heading",
maxTokens: 100,
});

expect(result.parser).toBe("heading");
expect(result.chunks.length).toBeGreaterThan(1);

// Every sub-chunk after the first should contain the heading
for (const chunk of result.chunks.slice(1)) {
expect(chunk.content).toContain("## Big Section");
}
});

it("splits at paragraph boundaries, not mid-paragraph", () => {
const para1 = "First paragraph with several words that make it meaningful.";
const para2 = "Second paragraph also has content that should stay together.";
const para3 = "Third paragraph is the final one in this section.";
const content = `## Section\n\n${para1}\n\n${para2}\n\n${para3}`;

const result = chunkFile({
filePath: "doc.md",
content,
strategy: "heading",
maxTokens: 50,
});

expect(result.parser).toBe("heading");

for (const chunk of result.chunks) {
// No paragraph should be cut in the middle — each paragraph
// should appear fully in exactly one chunk
if (chunk.content.includes("First paragraph")) {
expect(chunk.content).toContain(para1);
}

if (chunk.content.includes("Second paragraph")) {
expect(chunk.content).toContain(para2);
}

if (chunk.content.includes("Third paragraph")) {
expect(chunk.content).toContain(para3);
}
}
});

it("names preamble content before first heading", () => {
const content = `This is preamble text before any heading.\n\nMore preamble.\n\n## First Heading\n\nContent here.`;

const result = chunkFile({
filePath: "doc.md",
content,
strategy: "heading",
});

expect(result.parser).toBe("heading");

const preambleChunk = result.chunks[0];
expect(preambleChunk.name).toBeDefined();
expect(preambleChunk.name).not.toBe("chunk");
expect(preambleChunk.content).toContain("preamble text");
});

it("keeps small sections as single chunks", () => {
const content = `## Small Section\n\nJust a few words here.`;

const result = chunkFile({
filePath: "doc.md",
content,
strategy: "heading",
});

expect(result.parser).toBe("heading");
expect(result.chunks.length).toBe(1);
expect(result.chunks[0].name).toBe("Small Section");
expect(result.chunks[0].name).not.toContain("(part");
});

it("handles markdown with no headings at all", () => {
const content = `Just plain text.\n\nWith some paragraphs.\n\nAnd nothing else.`;

const result = chunkFile({
filePath: "doc.md",
content,
strategy: "heading",
});

expect(result.parser).toBe("heading");
expect(result.chunks.length).toBeGreaterThanOrEqual(1);
expect(result.chunks[0].name).toBeDefined();
});
});
});
128 changes: 126 additions & 2 deletions src/indexer/lib/chunker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,14 +608,117 @@ function chunkByLine(opts: { filePath: string; content: string; maxTokens: numbe
return { chunks, language, parser: "line" };
}

// ─── Paragraph-aware splitting for markdown ─────────────────────
function splitByParagraphs(opts: {
content: string;
filePath: string;
startLine: number;
kind: string;
name?: string;
maxTokens: number;
headingLine?: string;
}): ChunkRecord[] {
const { content, filePath, startLine, kind, name, maxTokens, headingLine } = opts;
const paragraphs = content.split(/\n\n+/);

if (paragraphs.length <= 1) {
return splitChunkByLines({
content,
filePath,
startLine,
kind,
name,
language: "markdown",
maxTokens,
});
}

const chunks: ChunkRecord[] = [];
let accumulated: string[] = [];
let accStartLine = startLine;

function flush(): void {
if (accumulated.length === 0) {
return;
}

const sourceContent = accumulated.join("\n\n");

if (sourceContent.trim().length === 0) {
return;
}

let chunkContent = sourceContent;

// Prepend heading to sub-chunks after the first for context
if (chunks.length > 0 && headingLine) {
chunkContent = `${headingLine}\n\n${sourceContent}`;
}

const sourceLineCount = sourceContent.split("\n").length;
const partLabel = chunks.length > 0 ? ` (part ${chunks.length + 1})` : "";

chunks.push({
id: xxhash(chunkContent),
filePath,
startLine: accStartLine,
endLine: accStartLine + sourceLineCount - 1,
content: chunkContent,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
kind,
name: name ? `${name}${partLabel}` : undefined,
language: "markdown",
});

accumulated = [];
}

let lineOffset = startLine;

for (const para of paragraphs) {
// If a single paragraph exceeds maxTokens, fall back to line splitting
if (accumulated.length === 0 && estimateTokens(para) > maxTokens) {
const paraChunks = splitChunkByLines({
content: headingLine ? `${headingLine}\n\n${para}` : para,
filePath,
startLine: lineOffset,
kind,
name,
language: "markdown",
maxTokens,
});
chunks.push(...paraChunks);
lineOffset += para.split("\n").length + 1;
accStartLine = lineOffset;
continue;
}

const candidate = [...accumulated, para].join("\n\n");
const withHeading = chunks.length > 0 && headingLine ? `${headingLine}\n\n${candidate}` : candidate;

if (estimateTokens(withHeading) > maxTokens && accumulated.length > 0) {
flush();
accStartLine = lineOffset;
}

accumulated.push(para);
lineOffset += para.split("\n").length + 1;
}

flush();

Comment thread
coderabbitai[bot] marked this conversation as resolved.
return chunks;
}

// ─── Heading strategy (markdown) ────────────────────────────────
function chunkByHeading(opts: { filePath: string; content: string; maxTokens: number }): ChunkResult {
const { filePath, content, maxTokens } = opts;
const lines = content.split("\n");
const chunks: ChunkRecord[] = [];
let currentLines: string[] = [];
let currentName: string | undefined;
let currentHeadingLine: string | undefined;
let currentStartLine = 0;
let seenHeading = false;

function flushSection(): void {
if (currentLines.length === 0) {
Expand All @@ -628,14 +731,28 @@ function chunkByHeading(opts: { filePath: string; content: string; maxTokens: nu
return;
}

const subChunks = splitChunkByLines({
if (estimateTokens(sectionContent) <= maxTokens) {
chunks.push({
id: xxhash(sectionContent),
filePath,
startLine: currentStartLine,
endLine: currentStartLine + currentLines.length - 1,
content: sectionContent,
kind: "heading",
name: currentName,
language: "markdown",
});
return;
}

const subChunks = splitByParagraphs({
content: sectionContent,
filePath,
startLine: currentStartLine,
kind: "heading",
name: currentName,
language: "markdown",
maxTokens,
headingLine: currentHeadingLine,
});

chunks.push(...subChunks);
Expand All @@ -649,8 +766,15 @@ function chunkByHeading(opts: { filePath: string; content: string; maxTokens: nu
flushSection();
currentLines = [line];
currentName = headingMatch[2].trim();
currentHeadingLine = line;
currentStartLine = i;
seenHeading = true;
} else {
// Name preamble content (before first heading)
if (!seenHeading && !currentName && line.trim().length > 0) {
currentName = line.trim().length > 50 ? `${line.trim().slice(0, 50)}...` : line.trim();
}

currentLines.push(line);
}
}
Expand Down
Loading
Loading