feat(ai): LLM tool/function calling — ai.agent() + built-in tools (#714)#1372
Conversation
Add the internal layer for LLM tool/function calling, shared across QuickAdd's three provider wire shapes (OpenAI-compatible, Anthropic Messages, Gemini generateContent): - NormalizedTools: provider-neutral request/message/tool-call types. - providerToolMapping: buildChatBody/parseChatResponse per provider, incl. OpenAI JSON-string arg parsing, Anthropic tool_result turns + native output_config.format (additionalProperties injected, as Anthropic requires), Gemini functionCall/Response + thoughtSignature echo, and max_completion_tokens for GPT-5.x/o-series reasoning models. - runToolLoop: pure bounded multi-step loop (forced-final text turn, one result per call id, step/result-byte/cumulative-transcript-byte caps, graceful context-overflow + online-disabled stops). - jsonSchemaValidator: registration-time keyword allowlist + value validation. - sanitizeVaultPath: every-segment dot-floor + traversal/absolute rejection. - OpenAIRequest: chatRequest() dispatch + Anthropic request-path fix; legacy single-shot routing unified on provider kind. - Provider: ProviderKind discriminator + getProviderKind() + migration.
) Expose the AI-SDK-shaped surface on quickAddApi.ai and ship opt-in built-in tools: - ai.agent(config).generate({ prompt, schema?, assignToVariable? }) — bounded tool loop + optional JSON-schema structured output (one bounded repair). - ai.tool(), ai.stepCountIs, ai.hasToolCall; assignToVariable alias on ai.prompt/ai.chunkedPrompt (reserved-name guarded, trimmed at write). - ai.tools.{vault,workspace,system}(options): reads auto-run; writes ask for approval, sanitise paths, fail-rather-than-overwrite, frontmatter-aware, symlink/realpath-contained. High-risk tools deferred. - Per-tool needsApproval + global ai.confirmToolCalls setting (default destructive; undefined→destructive for pre-existing settings) via AIToolConfirmModal (approve / approve-all-this-run / deny / abort). - EITHER-tools-OR-responseFormat per turn; per-run reentrancy guard.
A bundled script that wires up AI tool calling can let a model read/write the vault with model-chosen arguments. Scan the same decoded code the loader runs (including a js fence hidden in a .md note) and surface a distinct critical "AI TOOLS" capability row.
…714) Env-gated (skipped without each provider's API key, so CI never hits the network). Each exercises the real pure modules end-to-end against a current model: tool loop + schema-constrained structured output. Validated live on gpt-5-mini/gpt-5.5, claude-sonnet-4-6/claude-opus-4-8, gemini-3-flash-preview.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a multi-provider AI agent/tool-calling system to QuickAdd. Introduces ChangesAI Agent Tool Calling
Sequence Diagram(s)sequenceDiagram
participant Script
participant Agent
participant runToolLoop
participant chatRequest
participant AIToolConfirmModal
Script->>Agent: generate({ prompt, schema })
Agent->>Agent: buildSeedMessages + buildRegistry
Agent->>runToolLoop: RunToolLoopDeps
loop each turn
runToolLoop->>chatRequest: NormalizedChatRequest (tools or schema)
chatRequest-->>runToolLoop: ParsedChatResult
opt tool calls present
runToolLoop->>Agent: confirm(toolName, args)
Agent->>AIToolConfirmModal: Prompt(app, toolName, args)
AIToolConfirmModal-->>Agent: allow / deny / abort / allow-all
Agent-->>runToolLoop: outcome
runToolLoop->>runToolLoop: execute + stringifyToolResult
end
runToolLoop->>runToolLoop: append messages, check budget/stop
end
runToolLoop-->>Agent: RunToolLoopResult
opt schema provided
Agent->>Agent: parseAndValidate + optional repair re-ask
end
Agent-->>Script: GenerateResult
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Deploying quickadd with
|
| Latest commit: |
2612f25
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://aa226cd9.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-714-llm-function-c.quickadd.pages.dev |
CodeQL js/incomplete-url-substring-sanitization: endpoint.includes(host) could mis-route a URL with the known host in its path/query or as a fake subdomain prefix. Parse the hostname and match exactly or on a real subdomain instead.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/ai/tools/providerToolMapping.test.ts (1)
68-90: ⚡ Quick winAdd a regression case for non-object JSON tool arguments.
Current OpenAI parse tests cover valid-object JSON and malformed JSON, but not valid JSON that is the wrong shape (e.g.
"[]"or"42"). Adding this guards the normalized-call object-args contract.Suggested test addition
it("flags malformed JSON arguments with parseError (no throw)", () => { // ... }); + +it("flags non-object JSON arguments with parseError", () => { + const res = parseChatResponse("openai", { + choices: [{ + finish_reason: "tool_calls", + message: { tool_calls: [{ id: "c1", function: { name: "create_note", arguments: "[]" } }] }, + }], + }); + expect(res.toolCalls[0]).toMatchObject({ id: "c1", args: null, parseError: true }); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ai/tools/providerToolMapping.test.ts` around lines 68 - 90, Add a new test case to the providerToolMapping.test.ts file that tests the parseChatResponse function with valid JSON arguments that are not object-shaped (such as a string containing "[]" or "42"). The test should verify that when tool arguments are valid JSON but represent a non-object type, the tool call is handled appropriately by checking that either args is null or parseError is set to true, ensuring the normalized call object maintains its contract that args must be an object or null.src/ai/tools/runToolLoop.test.ts (1)
227-231: ⚡ Quick winAssert truncation against bytes, not characters.
This test currently checks character length only, so it won’t catch byte-cap regressions on multibyte output.
Suggested patch
it("truncates oversized results with a marker", () => { const r = stringifyToolResult("y".repeat(5000), 100); expect(r.content.endsWith("…[truncated]")).toBe(true); expect(r.content.length).toBeLessThan(5000); + expect(new TextEncoder().encode(r.content).length).toBeLessThanOrEqual(100); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ai/tools/runToolLoop.test.ts` around lines 227 - 231, The test "truncates oversized results with a marker" currently asserts character length using r.content.length instead of byte length, which won't catch regressions when multibyte characters are involved. Replace the character length assertion in the expect statement with a byte length check using Buffer.byteLength(r.content, 'utf-8') to properly validate that the truncation respects the byte limit and handles multibyte characters correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/docs/QuickAddAPI.md`:
- Around line 686-690: The blockquote starting with "Confirmation needs an
interactive Obsidian session" contains a blank line that violates the MD028
markdown linting rule. Remove the blank line entirely to keep the blockquote
contiguous, or if intentional spacing is needed, prefix the blank line with `>`
to maintain it as part of the blockquote structure.
In `@src/ai/Provider.ts`:
- Around line 41-47: The provider-kind inference in the condition checks at
lines 41 and 45 uses substring matching with endpoint.includes() which can
incorrectly match domain names that appear in URL paths or query parameters
rather than the actual hostname. To fix this, parse the endpoint URL to extract
the actual host/hostname (e.g., using URL constructor or similar parsing method)
and then perform exact comparisons against the target hostnames like
"api.anthropic.com" and "generativelanguage.googleapis.com". Apply this same
host-based matching approach to all similar provider detection conditions in the
setProviderKind method to ensure consistent and accurate provider
classification.
In `@src/ai/tools/Agent.ts`:
- Around line 222-231: Add a guard condition before calling
resolveStructuredObject() to ensure the loop completed successfully. The issue
is that resolveStructuredObject() is invoked even when the loop ended in failure
states (aborted or context-overflow), allowing the repair logic to make
unintended API calls that bypass safety guards. Before the if statement that
checks options.schema, add a condition to verify the loop did not abort or
encounter context-overflow, ensuring structured object resolution only proceeds
on successful loop completion. Apply this guard to all instances where
resolveStructuredObject() is called as noted in the diff range and the "Also
applies to" reference.
In `@src/ai/tools/builtins/vaultTools.ts`:
- Around line 164-165: The note writer paths are not consistently enforcing
markdown extension because the ensureMd function preserves existing non-.md
extensions instead of strictly enforcing them. Locate the ensureMd function
implementation (around line 252) and modify it to strip any existing file
extension before appending .md, ensuring all paths are strictly forced to
markdown format. Then apply this consistent enforcement across all note writer
path sanitization calls at lines 164, 184, and 211 where sanitizeVaultPath and
ensureMd are used together.
In `@src/ai/tools/jsonSchemaValidator.ts`:
- Around line 105-113: The assertRegisterableSchema function currently accepts
tuple-style items schemas (where schema.items is an array), but the runtime
validation logic only handles the case where items is a single schema, causing
tuple constraints to be registered but then ignored at validation time. Fix this
by either rejecting tuple-style items arrays in the registration check (the else
branch that currently calls assertRegisterableSchema recursively) or by adding
complete validation handling for arrays at the corresponding validation location
around line 188-194. The simplest fix is to throw an error in
assertRegisterableSchema when schema.items is an array, since tuple-style
validation is not supported in the codebase.
- Around line 170-175: The required property validation in the schema validation
loop uses the `in` operator with `key in obj`, which checks the entire prototype
chain including inherited properties. This causes false positives when checking
for required fields that exist on the prototype (like "toString"). Replace the
`key in obj` check with `Object.prototype.hasOwnProperty.call(obj, key)` to only
check for own properties actually present on the object, ensuring accurate
validation of required fields.
In `@src/ai/tools/providerToolMapping.ts`:
- Around line 201-226: The parseOpenAIToolCall function does not validate that
the args field is always an object, violating the normalized call contract.
After JSON.parse() on line 208, check if the parsed result is a plain object; if
not, return with args: null and parseError: true instead. Similarly, in the
final return statement on line 225, validate that the cast value is an actual
object before returning it, otherwise return with args: null and parseError:
true. This ensures args is consistently either a Record/object, null with
parseError flag, or explicitly null.
In `@src/ai/tools/runToolLoop.ts`:
- Around line 151-159: The truncation logic in the if block reduces the string
to fit within maxBytes, but then appends the " …[truncated]" marker which can
cause the final string to exceed the configured limit. Modify the truncation
logic to account for the byte length of the truncation marker: subtract the byte
length of " …[truncated]" from maxBytes before entering the while loop that
calculates the cut position, so that when the marker is appended to the sliced
string, the total result remains within the original maxBytes constraint.
In `@src/gui/AIToolConfirmModal.ts`:
- Around line 23-29: The static method `Prompt` in the AIToolConfirmModal class
violates the repository's camelCase naming convention for functions. Rename the
static method from `Prompt` to `prompt` (lowercase), and then search the entire
codebase for all call sites that invoke AIToolConfirmModal.Prompt and update
them to use AIToolConfirmModal.prompt instead.
In `@src/quickAddApi.ts`:
- Around line 415-417: The validation logic for assignToVariable uses a truthy
check which allows empty strings to bypass validation since empty strings are
falsy, but these invalid names can still be assigned later. Replace the truthy
check with an explicit check for undefined (such as `!== undefined` or checking
property existence) when calling assertAssignableVariableName so that all
provided values including empty strings are validated. Apply this change to all
instances where assignToVariable is validated: the ones near line 415-417, line
517-519, and any other similar patterns in the file where assignToVariable
validation occurs before it is used as outputVariableName.
---
Nitpick comments:
In `@src/ai/tools/providerToolMapping.test.ts`:
- Around line 68-90: Add a new test case to the providerToolMapping.test.ts file
that tests the parseChatResponse function with valid JSON arguments that are not
object-shaped (such as a string containing "[]" or "42"). The test should verify
that when tool arguments are valid JSON but represent a non-object type, the
tool call is handled appropriately by checking that either args is null or
parseError is set to true, ensuring the normalized call object maintains its
contract that args must be an object or null.
In `@src/ai/tools/runToolLoop.test.ts`:
- Around line 227-231: The test "truncates oversized results with a marker"
currently asserts character length using r.content.length instead of byte
length, which won't catch regressions when multibyte characters are involved.
Replace the character length assertion in the expect statement with a byte
length check using Buffer.byteLength(r.content, 'utf-8') to properly validate
that the truncation respects the byte limit and handles multibyte characters
correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7e91f0c-119d-4cef-96b6-e2e79d53a2e7
📒 Files selected for processing (36)
docs/docs/AIAssistant.mddocs/docs/QuickAddAPI.mdsrc/ai/OpenAIRequest.test.tssrc/ai/OpenAIRequest.tssrc/ai/Provider.test.tssrc/ai/Provider.tssrc/ai/tools/Agent.test.tssrc/ai/tools/Agent.tssrc/ai/tools/NormalizedTools.tssrc/ai/tools/aiToolTypes.tssrc/ai/tools/anthropic.e2e.test.tssrc/ai/tools/assignableVariable.tssrc/ai/tools/builtins/builtins.test.tssrc/ai/tools/builtins/shared.tssrc/ai/tools/builtins/systemTools.tssrc/ai/tools/builtins/vaultTools.tssrc/ai/tools/builtins/vaultWriteGuards.tssrc/ai/tools/builtins/workspaceTools.tssrc/ai/tools/gemini.e2e.test.tssrc/ai/tools/jsonSchemaValidator.test.tssrc/ai/tools/jsonSchemaValidator.tssrc/ai/tools/openai.e2e.test.tssrc/ai/tools/providerToolMapping.test.tssrc/ai/tools/providerToolMapping.tssrc/ai/tools/runToolLoop.test.tssrc/ai/tools/runToolLoop.tssrc/ai/tools/sanitizeVaultPath.test.tssrc/ai/tools/sanitizeVaultPath.tssrc/gui/AIAssistantSettingsModal.tssrc/gui/AIToolConfirmModal.tssrc/migrations/migrate.tssrc/migrations/setProviderKind.tssrc/quickAddApi.tssrc/services/packagePreview.test.tssrc/services/packagePreview.tssrc/settings.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dd9b8a5af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…714) Address CodeRabbit review on #1372: - jsonSchemaValidator: reject tuple-style `items` arrays at registration (they were accepted but skipped at runtime); use own-property check for `required` so an inherited name (e.g. "toString") can't satisfy it. - providerToolMapping: normalize OpenAI tool args — a parsed array/primitive now becomes parseError instead of a non-object `args`. - runToolLoop: reserve the truncation marker's bytes so a capped tool result (content + marker) still fits maxResultBytes. - vaultTools: note writers enforce .md — refuse a non-markdown extension so a model can't create/modify a .js (runnable) or other arbitrary file type. - Agent: don't run the structured-output network repair when the loop ended aborted/context-overflow or online features are disabled (keep the free first parse); the first parse never makes a call. - quickAddApi: an empty assignToVariable ("") now falls back instead of leaking as the output variable name, matching ai.agent's length>0 behavior. - docs: convert the confirmation note to a :::note admonition (markdownlint MD028).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ai/tools/builtins/builtins.test.ts (1)
78-88: ⚡ Quick winStrengthen the append writer safety assertion to match the test intent.
Line 87 only verifies
vault.createwasn’t called, which doesn’t coverappend_to_note. Add explicit assertions that append-path write/read APIs were not touched.Suggested test hardening
it("note writers refuse a non-markdown extension (no .js/.css etc.)", async () => { const app = makeApp(); const tools = createVaultTools(app); await expect( tools.create_note.execute({ path: "Notes/evil.js" }, { toolCallId: "c", toolName: "create_note" }), ).rejects.toThrow(/markdown/i); await expect( tools.append_to_note.execute({ path: "Notes/style.css", content: "x" }, { toolCallId: "c", toolName: "append_to_note" }), ).rejects.toThrow(/markdown/i); expect((app.vault.create as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled(); + expect((app.vault.modify as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled(); + expect((app.vault.read as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ai/tools/builtins/builtins.test.ts` around lines 78 - 88, The test is only asserting that app.vault.create was not called, but it does not verify that the vault append/write methods were not called when tools.append_to_note.execute is invoked with an invalid CSS extension. Add explicit assertions after the expect block for append_to_note to verify that the vault append and read methods (such as app.vault.append or app.vault.modify) were not called, ensuring the test properly validates that append_to_note refused to write non-markdown files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/ai/tools/builtins/builtins.test.ts`:
- Around line 78-88: The test is only asserting that app.vault.create was not
called, but it does not verify that the vault append/write methods were not
called when tools.append_to_note.execute is invoked with an invalid CSS
extension. Add explicit assertions after the expect block for append_to_note to
verify that the vault append and read methods (such as app.vault.append or
app.vault.modify) were not called, ensuring the test properly validates that
append_to_note refused to write non-markdown files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ed8ed5b-4aef-4a66-b476-025abb14c114
📒 Files selected for processing (10)
docs/docs/QuickAddAPI.mdsrc/ai/tools/Agent.tssrc/ai/tools/builtins/builtins.test.tssrc/ai/tools/builtins/vaultTools.tssrc/ai/tools/jsonSchemaValidator.test.tssrc/ai/tools/jsonSchemaValidator.tssrc/ai/tools/providerToolMapping.test.tssrc/ai/tools/providerToolMapping.tssrc/ai/tools/runToolLoop.tssrc/quickAddApi.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/ai/tools/providerToolMapping.ts
- src/ai/tools/jsonSchemaValidator.test.ts
- src/ai/tools/builtins/vaultTools.ts
- src/ai/tools/jsonSchemaValidator.ts
- src/ai/tools/providerToolMapping.test.ts
- src/quickAddApi.ts
- docs/docs/QuickAddAPI.md
- src/ai/tools/Agent.ts
- src/ai/tools/runToolLoop.ts
…tion (#714) Address Codex review on #1372: - anthropicMaxTokens: default back to 4096 (the long-standing pre-#714 value). Anthropic 400s when max_tokens exceeds a model's OUTPUT cap (verified live), and 8192 exceeds Claude 3 Haiku's 4096 — so the bump could reject small-input requests. Explicit maxOutputTokens is still honored uncapped for 4.x models. - Remove the setProviderKind migration: it persisted an inferred wire kind that the provider editor can't change, so repointing a provider's endpoint left a stale kind and the wrong wire shape. getProviderKind already infers at request time when kind is absent; the explicit field stays for known/preset providers (e.g. a custom-named Anthropic proxy).
…tion (#714) Address Codex review on #1372: - anthropicMaxTokens: default back to 4096 (the long-standing pre-#714 value). Anthropic 400s when max_tokens exceeds a model's OUTPUT cap (verified live), and 8192 exceeds Claude 3 Haiku's 4096 — so the bump could reject small-input requests. Explicit maxOutputTokens is still honored uncapped for 4.x models. - Remove the setProviderKind migration: it persisted an inferred wire kind that the provider editor can't change, so repointing a provider's endpoint left a stale kind and the wrong wire shape. getProviderKind already infers at request time when kind is absent; the explicit field stays for known/preset providers (e.g. a custom-named Anthropic proxy).
47a4afb to
2612f25
Compare
Closes #714.
Adds LLM tool / function calling to QuickAdd's AI Assistant, exposed through the script API as an AI-SDK-shaped agent primitive. You give a model a prompt plus a set of tools (JS functions); it calls them in a bounded multi-step loop until it has an answer. Optional JSON-schema structured output is included.
Surface (
quickAddApi.ai)ai.agent(config).generate({ prompt, schema?, assignToVariable? })— bounded tool loop; withschema, returns a validatedobject(one bounded repair re-ask).ai.tool({ description, inputSchema, execute, needsApproval?, readOnly?, strict? })— declare your own tool.ai.tools.{vault,workspace,system}(options)— opt-in built-in tools (only/exclude/prefix/allowedRoots).ai.stepCountIs,ai.hasToolCall;assignToVariablealias added toai.prompt/ai.chunkedPrompt.Built-in tools: reads (
read_note,list_notes,search_notes,get_property_values,get_active_note,get_selection,get_date) auto-run; writes (create_note,append_to_note,insert_under_heading) ask for approval. High-risk tools (run_choice/apply_template/set_frontmatter/delete_note) are intentionally not shipped in v1.Design
src/ai/tools/, no Obsidian imports) maps to/from the three wire shapes — OpenAI-compatible, Anthropic Messages, GeminigenerateContent— so the loop + mappers are fully unit-testable. The Obsidian-boundAgentis a thin shell over it.needsApproval+ global Confirm AI tool calls setting (default destructive only), step / per-result-byte / cumulative-transcript-byte caps, per-iteration online-disabled recheck, JSON-schema arg validation, path sanitisation + symlink/realpath vault containment, and never running model args throughformat(). Package-preview discloses anAI TOOLScapability for any bundled script that uses this.Validation
gpt-5-mini&gpt-5.5, Anthropicclaude-opus-4-8&claude-sonnet-4-6, Geminigemini-3-flash-preview(incl. the Gemini-3thoughtSignatureecho round-trip). e2e is env-gated, so CI never hits the network.max_completion_tokens(notmax_tokens); Anthropicoutput_config.formatrequiresadditionalProperties:falseinjected into the schema.tsc+ eslint clean; bundle 1.1 mb.Notes
Next) version only.Summary by CodeRabbit
Release Notes
New Features
gemini-3-pro,gemini-3-flash)Documentation