Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Expands the “Create Document” modal to support generating the draft via backend-assisted workflows (pulling values from related documents and executing a server-side script), with AI support for both document and script drafting.
Changes:
- Adds a two-tab Create Document UI (“Manual” and “Server script”), including pull-from-related mapping UI and script execution output/logs.
- Introduces backend actions for pulling values from related documents and executing a “create draft” script in a VM sandbox.
- Extends AI chat context/prompts to support generating either document bodies or server-side scripts.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/models/models.js | Stores modelSchemaPaths from listModels() to power path suggestions in Create Document. |
| frontend/src/models/models.html | Passes modelSchemaPaths into the Create Document component; widens modal via containerClass. |
| frontend/src/modal/modal.css | Adds a create-document-specific modal sizing/padding variant. |
| frontend/src/create-document/create-document.js | Implements pull mappings, backend pull merge, and server-script execution for draft generation; AI targeting document vs script. |
| frontend/src/create-document/create-document.html | Reworks Create Document modal UI into tabs and adds pull mapping + server script editors. |
| frontend/src/api.js | Adds API helpers for pullDocumentValues and executeCreateDocumentScript. |
| backend/authorize.js | Wires role requirements for the two new Model actions. |
| backend/actions/Model/streamChatMessage.js | Adds aiTarget + createDraftScript context and separate system prompts for document vs script generation. |
| backend/actions/Model/pullDocumentValues.js | New action: fetches related docs and extracts values by path into a returned object. |
| backend/actions/Model/index.js | Exports the new Model actions. |
| backend/actions/Model/executeCreateDocumentScript.js | New action: executes user-provided script in VM against a mutable draft object and returns updated draft + logs. |
| backend/actions/Model/createChatMessage.js | Mirrors streamChatMessage changes for non-streamed LLM calls (document vs script targeting). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <ace-editor | ||
| v-model="documentData" | ||
| mode="javascript" | ||
| :line-numbers="true" | ||
| class="h-[300px] w-full mt-2" | ||
| /> |
There was a problem hiding this comment.
The document editor is rendered twice (one Ace editor in the Manual tab and another in the Server script tab) but both bind to the same documentData. This creates and maintains two Ace instances, which is unnecessarily heavy and can lead to subtle sync/selection issues. Consider rendering a single shared ace-editor for documentData outside the tab panels (or use v-if to mount only the active tab’s editor).
| <ace-editor | |
| v-model="documentData" | |
| mode="javascript" | |
| :line-numbers="true" | |
| class="h-[300px] w-full mt-2" | |
| /> | |
| <pre | |
| class="mt-2 h-[300px] w-full overflow-auto rounded-md border border-edge-muted bg-muted/20 p-2 font-mono text-xs text-content-secondary whitespace-pre-wrap" | |
| >{{ documentData }}</pre> |
| throw new Error(`Model ${sourceModel} not found`); | ||
| } | ||
| const doc = await Model.findById(sourceDocumentId).setOptions({ sanitizeFilter: true }).orFail(); | ||
| const lean = doc.toObject({ virtuals: false, getters: true }); |
There was a problem hiding this comment.
pullDocumentValues() uses doc.toObject({ virtuals: false, getters: true }), which differs from getDocuments/getDocument (they use getters: false, transform: false). Using getters and default transforms can return computed/transformed values instead of raw stored data, which is surprising for a “pull field values” API. Consider using the same serialization options as the other Model read endpoints (e.g., toJSON({ virtuals: false, getters: false, transform: false })).
| const lean = doc.toObject({ virtuals: false, getters: true }); | |
| const lean = doc.toJSON({ virtuals: false, getters: false, transform: false }); |
| const values = {}; | ||
| for (const pull of pulls) { | ||
| const lean = await getLean(pull.sourceModel.trim(), pull.sourceDocumentId.trim()); | ||
| const v = mpath.get(pull.sourcePath, lean); | ||
| if (v !== undefined) { | ||
| mpath.set(pull.targetPath, v, values); | ||
| } |
There was a problem hiding this comment.
mpath.set(pull.targetPath, v, values) writes into a plain object using a user-supplied path. Without guarding against paths like __proto__, constructor, or prototype, this can enable prototype pollution on the server. Validate/sanitize targetPath (and potentially sourcePath) to reject dangerous segments, or build values with Object.create(null) and explicitly block these keys before calling mpath.set.
| const context = vm.createContext(sandbox); | ||
| const result = await vm.runInContext(wrappedScript(script), context); | ||
|
|
There was a problem hiding this comment.
User-provided scripts are executed via vm.runInContext() with no execution time limit. A script with an infinite loop (or heavy synchronous work) can block the Node event loop and take down the server process. Consider running these scripts in a worker thread/isolated process with an enforced timeout and resource limits (or otherwise hard-limiting execution) before exposing this endpoint broadly.
| module.exports = ({ db }) => async function pullDocumentValues(params) { | ||
| const { pulls, roles } = new PullDocumentValuesParams(params); | ||
|
|
||
| await authorize('Model.pullDocumentValues', roles); | ||
|
|
||
| if (!Array.isArray(pulls) || pulls.length === 0) { | ||
| throw new Error('pulls must be a non-empty array'); | ||
| } |
There was a problem hiding this comment.
New backend behavior (Model.pullDocumentValues) isn’t covered by tests. There are existing test/Model.*.test.js suites for Model actions; please add tests for at least: (1) pulling a simple field, (2) nested path pulls, (3) caching behavior for repeated pulls from the same source doc, and (4) rejecting invalid pull payloads.
| module.exports = ({ db }) => async function executeCreateDocumentScript(params) { | ||
| const { model, data, script, roles } = new ExecuteCreateDocumentScriptParams(params); | ||
|
|
||
| await authorize('Model.executeCreateDocumentScript', roles); | ||
|
|
||
| const Model = db.models[model]; | ||
| if (Model == null) { | ||
| throw new Error(`Model ${model} not found`); | ||
| } | ||
|
|
||
| const draft = EJSON.deserialize(data); |
There was a problem hiding this comment.
New backend behavior (Model.executeCreateDocumentScript) isn’t covered by tests. Please add a Model action test that verifies: (1) the script can mutate draft, (2) logs are captured, and (3) authorization/invalid model handling. This will help prevent regressions in the sandbox contract.
vkarpov15
left a comment
There was a problem hiding this comment.
Let's remove the Manual vs Server script distinction and the "Pull from another document" input, in favor of allowing Model/createChatMessage to execute some lightweight tools to pull data for the user using the tools in #201
The idea is that, as a user creating an AccessToken, I should be able to say "create token for user with email val@karpov.io" and the LLM can execute a query for User with email val@karpov.io via a tool, and generate the draft document with the correct id.
| documentData: { | ||
| $type: 'string' | ||
| }, | ||
| createDraftScript: { |
There was a problem hiding this comment.
Model/streamChatMessage is explicitly for chat threads, we shouldn't use it for generating documents.
| class="mb-4 rounded-md border border-emerald-200 bg-emerald-50/70 p-3 dark:border-emerald-900 dark:bg-emerald-950/40" | ||
| > | ||
| <p class="mb-2 text-xs text-content-secondary"> | ||
| AI suggestion ready for the <strong class="text-content">{{ lastAiTarget === 'script' ? 'server script' : 'document' }}</strong>. Accept to keep it or reject to restore the previous version. |
There was a problem hiding this comment.
It is a little messy how the "AI suggestion" shows up at the bottom below everything, instead of right under AI mode
| /> | ||
| </div> | ||
| <div class="rounded-md border border-edge-strong bg-page0 p-3"> | ||
| <label class="block text-sm font-bold text-content">Pull from another document</label> |
There was a problem hiding this comment.
The "Pull from another document" is unfortunately quite clunky - the idea is that the AI should be able to fetch data on its own to give back to the user. Take a look at the agents mode PR for how you can use the AI SDK to expose tools for the AI to pull data: #201.
The use case I want to support is this: in the AccessToken model, I should be able to enter in "create token for user with email val@karpov.io" and the AI should pull the ObjectId for user with email val@karpov.io for me, and insert it into the Document to Create.
| <button | ||
| type="button" | ||
| role="tab" | ||
| :aria-selected="createModalTab === 'manual'" |
There was a problem hiding this comment.
Manual vs Server script distinction is unclear here, I'd prefer to remove the two tabs entirely and just expand it so AI Mode can handle pulling data
|
Going to close this for now until we can get agent mode out for better guidance on how to implement this |
closes #177