Skip to content

feat: wire document update to MCP, CLI, and REST API (#182)#270

Merged
RobertLD merged 2 commits intomainfrom
feat/document-update
Mar 2, 2026
Merged

feat: wire document update to MCP, CLI, and REST API (#182)#270
RobertLD merged 2 commits intomainfrom
feat/document-update

Conversation

@RobertLD
Copy link
Owner

@RobertLD RobertLD commented Mar 2, 2026

Closes #182

Exposes the existing updateDocument() function through all interfaces:

  • MCP tool: update-document — update title, content, library, version, url, topicId
  • CLI: libscope docs update <id> with --title, --content, --library, --version, --url, --topic
  • REST API: PUT/PATCH /api/v1/documents/:id — partial updates
  • Content changes trigger automatic re-chunking and re-embedding
  • 5 new tests for update flows

Copilot AI review requested due to automatic review settings March 2, 2026 20:02
@vercel
Copy link

vercel bot commented Mar 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
libscope Ready Ready Preview, Comment Mar 2, 2026 8:13pm

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for updating existing documents across LibScope’s interfaces (core export, MCP tool, CLI command, and REST API), including re-chunking/re-embedding when content changes.

Changes:

  • Add updateDocument to public core exports and wire it into MCP and CLI.
  • Add REST PUT/PATCH /api/v1/documents/:id route to perform partial document updates.
  • Add unit tests and CLI reference docs for the new update flow.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/unit/documents.test.ts Adds unit tests covering updateDocument update/validation scenarios
src/mcp/server.ts Exposes update-document MCP tool and maps parameters to updateDocument
src/core/index.ts Exports updateDocument and UpdateDocumentInput from core
src/cli/index.ts Adds libscope docs update <id> command that calls updateDocument
src/api/routes.ts Adds PUT/PATCH handler for /api/v1/documents/:id using updateDocument
docs/reference/cli.md Documents the new CLI docs update command

Comment on lines +243 to +249
withErrorHandling(async (params) => {
const metadata: Record<string, string | null | undefined> = {};
if (params.library !== undefined) metadata.library = params.library;
if (params.version !== undefined) metadata.version = params.version;
if (params.url !== undefined) metadata.url = params.url;
if (params.topicId !== undefined) metadata.topicId = params.topicId;

Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

This tool allows calls with only documentId and no update fields; that will still create a new version / bump timestamps even though nothing changed. Consider validating that at least one of title, content, or the metadata fields is provided and return a ValidationError otherwise.

Copilot uses AI. Check for mistakes.
Comment on lines 116 to 117
## Document Links (Cross-references)

Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The libscope docs update docs are inserted under the “Document Links (Cross-references)” section and also cause that header to appear twice. This should likely live under “Document Management” (near list/show/delete/history) and the duplicate “Document Links” header should be removed to keep the structure consistent.

Suggested change
## Document Links (Cross-references)

Copilot uses AI. Check for mistakes.
Comment on lines +164 to +173
const updated = await updateDocument(vecDb, provider, indexed.id, {
content: "Brand new content that is different.",
});

expect(updated.content).toBe("Brand new content that is different.");
// Verify chunks were recreated
const chunks = vecDb
.prepare("SELECT id FROM chunks WHERE document_id = ?")
.all(indexed.id) as { id: string }[];
expect(chunks.length).toBeGreaterThan(0);
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The “should update content and re-chunk” assertion only checks that at least one chunk exists after the update, which would also pass if updateDocument did not delete/recreate chunks (because chunks already exist from the initial index). Strengthen this test to verify re-chunking actually happened (e.g., compare old vs new chunk IDs/content, verify chunk count changes appropriately, and/or confirm chunk_embeddings rows were replaced).

Suggested change
const updated = await updateDocument(vecDb, provider, indexed.id, {
content: "Brand new content that is different.",
});
expect(updated.content).toBe("Brand new content that is different.");
// Verify chunks were recreated
const chunks = vecDb
.prepare("SELECT id FROM chunks WHERE document_id = ?")
.all(indexed.id) as { id: string }[];
expect(chunks.length).toBeGreaterThan(0);
// Capture original chunks and their embeddings
const originalChunks = vecDb
.prepare(
"SELECT id, content FROM chunks WHERE document_id = ? ORDER BY id",
)
.all(indexed.id) as { id: string; content: string }[];
expect(originalChunks.length).toBeGreaterThan(0);
const originalChunkIds = originalChunks.map((c) => c.id);
const originalEmbeddings = vecDb
.prepare(
"SELECT chunk_id FROM chunk_embeddings WHERE document_id = ? ORDER BY chunk_id",
)
.all(indexed.id) as { chunk_id: string }[];
expect(originalEmbeddings.length).toBeGreaterThan(0);
const updated = await updateDocument(vecDb, provider, indexed.id, {
content: "Brand new content that is different.",
});
expect(updated.content).toBe("Brand new content that is different.");
// Verify chunks were recreated: new chunks exist and have different IDs
const newChunks = vecDb
.prepare(
"SELECT id, content FROM chunks WHERE document_id = ? ORDER BY id",
)
.all(indexed.id) as { id: string; content: string }[];
expect(newChunks.length).toBeGreaterThan(0);
const newChunkIds = newChunks.map((c) => c.id);
// Ensure none of the original chunk IDs are reused
expect(
newChunkIds.every((id) => !originalChunkIds.includes(id)),
).toBe(true);
// Verify embeddings now correspond to the new chunks
const newEmbeddings = vecDb
.prepare(
"SELECT chunk_id FROM chunk_embeddings WHERE document_id = ? ORDER BY chunk_id",
)
.all(indexed.id) as { chunk_id: string }[];
expect(newEmbeddings.length).toBeGreaterThan(0);
const newEmbeddingChunkIds = newEmbeddings.map((e) => e.chunk_id);
const sortedNewChunkIds = [...newChunkIds].sort();
const sortedNewEmbeddingChunkIds = [...newEmbeddingChunkIds].sort();
expect(sortedNewEmbeddingChunkIds).toEqual(sortedNewChunkIds);

Copilot uses AI. Check for mistakes.
sendError(res, 400, "VALIDATION_ERROR", "Request body must be a JSON object");
return;
}
const b = body as Record<string, unknown>;
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

PUT/PATCH currently accept an empty JSON object (or a body with no supported fields) and still call updateDocument, which will create a new version / update timestamps despite being a no-op. Consider validating that at least one updatable field is present and return 400 for empty updates (matches the issue’s “invalid/empty body” requirement).

Suggested change
const b = body as Record<string, unknown>;
const b = body as Record<string, unknown>;
const hasUpdatableField =
"title" in b ||
"content" in b ||
"library" in b ||
"version" in b ||
"url" in b ||
"topicId" in b;
if (!hasUpdatableField) {
sendError(
res,
400,
"VALIDATION_ERROR",
"Request body must contain at least one updatable field",
);
return;
}

Copilot uses AI. Check for mistakes.
Comment on lines +420 to +426
if (b["library"] !== undefined)
metadata.library = typeof b["library"] === "string" ? b["library"] : null;
if (b["version"] !== undefined)
metadata.version = typeof b["version"] === "string" ? b["version"] : null;
if (b["url"] !== undefined) metadata.url = typeof b["url"] === "string" ? b["url"] : null;
if (b["topicId"] !== undefined)
metadata.topicId = typeof b["topicId"] === "string" ? b["topicId"] : null;
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

For library/version/url/topicId, if the field is present but not a string (e.g. 123), this code coerces it to null, which will silently clear the stored value. It would be safer to treat “present but not string or null” as a validation error (400) to avoid unintended data loss.

Suggested change
if (b["library"] !== undefined)
metadata.library = typeof b["library"] === "string" ? b["library"] : null;
if (b["version"] !== undefined)
metadata.version = typeof b["version"] === "string" ? b["version"] : null;
if (b["url"] !== undefined) metadata.url = typeof b["url"] === "string" ? b["url"] : null;
if (b["topicId"] !== undefined)
metadata.topicId = typeof b["topicId"] === "string" ? b["topicId"] : null;
const library = b["library"];
if (library !== undefined) {
if (library !== null && typeof library !== "string") {
sendError(res, 400, "VALIDATION_ERROR", "Field 'library' must be a string or null");
return;
}
metadata.library = library as string | null;
}
const version = b["version"];
if (version !== undefined) {
if (version !== null && typeof version !== "string") {
sendError(res, 400, "VALIDATION_ERROR", "Field 'version' must be a string or null");
return;
}
metadata.version = version as string | null;
}
const url = b["url"];
if (url !== undefined) {
if (url !== null && typeof url !== "string") {
sendError(res, 400, "VALIDATION_ERROR", "Field 'url' must be a string or null");
return;
}
metadata.url = url as string | null;
}
const topicId = b["topicId"];
if (topicId !== undefined) {
if (topicId !== null && typeof topicId !== "string") {
sendError(res, 400, "VALIDATION_ERROR", "Field 'topicId' must be a string or null");
return;
}
metadata.topicId = topicId as string | null;
}

Copilot uses AI. Check for mistakes.
Comment on lines +718 to +723
const { db, provider } = initializeAppWithEmbedding();
try {
const metadata: Record<string, string | null | undefined> = {};
if (opts.library !== undefined) metadata.library = opts.library;
if (opts.version !== undefined) metadata.version = opts.version;
if (opts.url !== undefined) metadata.url = opts.url;
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The CLI command can be invoked with no flags (only <documentId>), which will call updateDocument with no changes and still create a new version / bump updated_at. Add a guard to require at least one of the update options and show a helpful error when none are provided.

Copilot uses AI. Check for mistakes.
RobertLD and others added 2 commits March 2, 2026 20:12
@RobertLD RobertLD force-pushed the feat/document-update branch from 53cf4e3 to 49759ff Compare March 2, 2026 20:13
@RobertLD RobertLD merged commit 96ced04 into main Mar 2, 2026
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document UPDATE endpoints (PUT/PATCH)

2 participants