fix: clamp negative fromLine in get to avoid silent tail content
The query tool description tells agents to compute fromLine = line - 20 for context around a hit. For hits in lines 1 through 20 that yields a negative fromLine, which propagated unchanged through: MCP get handler -> store.getDocumentBody -> Array.prototype.slice A negative slice start offsets from the end of the array rather than clamping to the beginning, so a top-of-file hit on a long document returned an empty string and on a short document returned content from the wrong region (e.g. lines 11-30 of a 30-line file in response to a request for the head of the document). The lineNumbers branch was the same shape: addLineNumbers(text, -19) emitted "-19:", "-18:" prefixes. Same buggy slice lived in the CLI getDocument path independently. Fix in three layers, plus the docstring: - src/mcp/server.ts: clamp parsedFromLine to >= 1 after parsing input args and the :line suffix, before it reaches getDocumentBody and addLineNumbers. Also tighten the query tool's recommendation to `fromLine = max(1, line - 20)` so following the docstring literally produces a valid value. - src/cli/qmd.ts: same clamp on the CLI getDocument fromLine after the colon-suffix parse. - src/store.ts: defensive Math.max(0, ...) on the slice start in getDocumentBody so SDK callers and any future entry points are protected without relying on every caller remembering to clamp. - test/store.test.ts: regression test on getDocumentBody with fromLine = -19 returns the head of the document, not the tail. - test/cli.test.ts: regression test on `qmd get --from -19` matches the no-flag baseline (head of document).
This commit is contained in:
parent
1f522cffe2
commit
aa1818e181
@ -844,6 +844,7 @@ function getDocument(filename: string, fromLine?: number, maxLines?: number, lin
|
||||
inputPath = inputPath.slice(0, -colonMatch[0].length);
|
||||
}
|
||||
}
|
||||
if (fromLine !== undefined) fromLine = Math.max(1, fromLine);
|
||||
|
||||
const parsedIndexPath = isVirtualPath(inputPath) ? parseVirtualPath(inputPath) : null;
|
||||
if (parsedIndexPath?.indexName) {
|
||||
|
||||
@ -243,7 +243,7 @@ async function createMcpServer(store: QMDStore): Promise<McpServer> {
|
||||
title: "Query",
|
||||
description: `Search the knowledge base using a query document — one or more typed sub-queries combined for best recall.
|
||||
|
||||
Each result includes a \`line\` field with the absolute 1-indexed line of the best match in the source markdown. To read more context around a hit, call \`get(file, fromLine = line - 20, maxLines = 80, lineNumbers = true)\`.
|
||||
Each result includes a \`line\` field with the absolute 1-indexed line of the best match in the source markdown. To read more context around a hit, call \`get(file, fromLine = max(1, line - 20), maxLines = 80, lineNumbers = true)\`.
|
||||
|
||||
## Query Types
|
||||
|
||||
@ -389,6 +389,7 @@ Intent-aware lex (C++ performance, not sports):
|
||||
parsedFromLine = parseInt(colonMatch[1], 10);
|
||||
lookup = lookup.slice(0, -colonMatch[0].length);
|
||||
}
|
||||
if (parsedFromLine !== undefined) parsedFromLine = Math.max(1, parsedFromLine);
|
||||
|
||||
const result = await store.get(lookup, { includeBody: false });
|
||||
|
||||
|
||||
@ -3800,7 +3800,7 @@ export function getDocumentBody(db: Database, doc: DocumentResult | { filepath:
|
||||
let body = row.body;
|
||||
if (fromLine !== undefined || maxLines !== undefined) {
|
||||
const lines = body.split('\n');
|
||||
const start = (fromLine || 1) - 1;
|
||||
const start = Math.max(0, (fromLine || 1) - 1);
|
||||
const end = maxLines !== undefined ? start + maxLines : lines.length;
|
||||
body = lines.slice(start, end).join('\n');
|
||||
}
|
||||
|
||||
@ -542,6 +542,13 @@ describe("CLI Get Command", () => {
|
||||
// Should indicate file not found
|
||||
expect(exitCode).toBe(1);
|
||||
});
|
||||
|
||||
test("clamps negative --from to top of file (no silent tail content)", async () => {
|
||||
const baseline = await runQmd(["get", "README.md"]);
|
||||
const negative = await runQmd(["get", "README.md", "--from", "-19"]);
|
||||
expect(negative.exitCode).toBe(0);
|
||||
expect(negative.stdout).toBe(baseline.stdout);
|
||||
});
|
||||
});
|
||||
|
||||
describe("CLI Multi-Get Command", () => {
|
||||
|
||||
@ -1713,6 +1713,21 @@ describe("Document Retrieval", () => {
|
||||
expect(body).toBeNull();
|
||||
await cleanupTestDb(store);
|
||||
});
|
||||
|
||||
test("getDocumentBody clamps negative fromLine to top of document", async () => {
|
||||
const store = await createTestStore();
|
||||
const collectionName = await createTestCollection({ pwd: "/path" });
|
||||
await insertTestDocument(store.db, collectionName, {
|
||||
name: "mydoc",
|
||||
displayPath: "mydoc.md",
|
||||
body: "Line 1\nLine 2\nLine 3\nLine 4\nLine 5",
|
||||
});
|
||||
|
||||
const body = store.getDocumentBody({ filepath: "/path/mydoc.md" }, -19, 80);
|
||||
expect(body).toBe("Line 1\nLine 2\nLine 3\nLine 4\nLine 5");
|
||||
|
||||
await cleanupTestDb(store);
|
||||
});
|
||||
});
|
||||
|
||||
describe("findDocuments (multi-get)", () => {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user