QVAC-13559 feat[api]: sdk "dynamic" tools mode by mialso · Pull Request #745 · tetherto/qvac

mialso · 2026-03-06T11:14:47Z

Description

Adds support for dynamic tools mode with @qvac/llm-llamacpp addon

Changes:

package.json: Upgraded @qvac/llm-llamacpp to ^0.17.0
new toolsMode configuration inteface
- 'static' mode (default): Tools are prepended to history (existing behavior)
- 'dynamic' mode: Tools exist in a user prompt scope and then removed from kv-cache
the rational behind static/dynamic naming is for the consumer (app using sdk) to understand the difference from the usage perspective, while implementation right now uses append/prepend (tools_compact addon param - how cache works). So a possible impl change won't affect public API
unit tests, examples

how to check it works:

bun install
bun run build
bun run ./examples/llamacpp-dynamic-tools.ts
bun run ./examples/agentic-tools.ts

addon implementation

github-actions · 2026-03-23T12:27:09Z

Tier-based Approval Status

**PR Tier:** TIER1

**Current Status:** ❌ PENDING

**Requirements:**
- 1 Team Member approval ❌ (0/1)
- 1 Team Lead OR Management approval ❌ (0/1)



---
*This comment is automatically updated when reviews change.*

opaninakuffo · 2026-03-23T15:16:40Z

@@ -0,0 +1,202 @@
+/* eslint-disable */
+// @ts-nocheck


@opaninakuffo initially I just copied sdk/examples/llamacpp-native-tools.ts and replaced the logic with "dynamic" tools handling - should remove so to pass both checks?

removed comments, now both lint and type checks pass ✔️

opaninakuffo · 2026-03-23T15:17:20Z

+      properties: Record<string, unknown>;
+      required?: string[];
+    };
+  }>,


we have numerous existing tool tests. Can we modify some of those to be dynamic instead?

I've assumed current tools tests cover available logic and if removed it would end with decreased coverage - I mean to remove a test in order to replace with a dynamic tools one it's required to "prove" it's useless, because a newly added "dynamic" tools one would go another code path 🤔
I will take a look if any test do same stuff, if possible to merge

improved tests:

extended available createToolsTest function to reuse with toolsMode param (removed prev added helper)

removed toolModeUnset test case as already covered by e.g. toolsSimpleFunction

…ic-tools-interface-support

NamelsKing · 2026-03-24T10:21:31Z

please update bun.lock

…ic-tools-interface-support

simon-iribarren · 2026-03-25T11:02:37Z

    const modelConfig = getModelConfig(modelId);
    const systemPromptFromHistory = extractSystemPrompt(history);
-    const configHash = generateConfigHash(systemPromptFromHistory, tools);
+    const toolsModeForHash = (modelConfig as { toolsMode?: string }).toolsMode;


isn't this the same than the toolsMode constant in the outer scope?

@simon-iribarren yes, I but I've decided to follow current logic since there are 2 model configs right now and kvCache "branch" has it's own, like

const modelConfig = getModelConfig(modelId); // <...> if (kvCache) { const modelConfig = getModelConfig(modelId); // at this point using toolsMode from the "second" config

hence this is just for consistency - otherwise prob we should refactor to have a single modelConfig?

…ic-tools-interface-support

…mic-tools

@ts-expect-error

- Remove `CompletionDebugStats` schema and `debugStats` from `CompletionRun`, `statsEventSchema`, and `buildStreamResult`. - Drop the runtime-debug-stats extraction and the `// @ts-expect-error test-error` workaround in `completion-stream.ts`. Those required an addon-side patch that is not in scope here. - Delete the `agentic-tools.ts` example. It was the only consumer of the debug-stats fields and the source of CodeQL findings (ReDoS on `<think>`/`<tool_call>`, double-unescape, weak URL hostname check). `llamacpp-dynamic-tools.ts` remains as the canonical dynamic-tools example.

- The PR introduced `validation: "custom"` + a `validator` callback in tools tests, but `@tetherto/qvac-test-suite@0.6.0`'s `Expectation` union does not include `"custom"`. Add a local `ToolsExpectation` extension that augments `Expectation` with the custom-validator shape, used by both the test definitions and the executor. The `TestDefinition` cast at construction keeps the runtime payload compatible with the test framework. - Two `createToolsTest` call sites (`toolsSimpleFunction`, `toolsMultipleFunctions`) passed `["smoke"]` as the 5th positional arg, which collides with the `expectation` slot. Add the explicit `undefined` between `toolsMode` and the `suites` argument so the array reaches the `suites` parameter. - The executor's `"custom"` branch now invokes the validator directly and shapes a `TestResult`, instead of forwarding to `ValidationHelpers.validate` (which only handles the built-in validation kinds). Both issues only surfaced once the `test-e2e-smoke` label was applied — the e2e workflow's tests-qvac typecheck leg is label-gated, so prior pushes never typechecked these files.

      stream?: boolean;
    };
-    const toolsModelId = await this.resources.ensureLoaded("tools");
+    const resourceDep = p.toolsMode  === ToolsModeType.dynamic ? "tools-dynamic" : "tools"


The plugin's `transformLlmConfig` was missing the SDK-to-addon translation for `toolsMode`. Without it, the addon receives `tools_mode` as a CLI-style flag, which it does not recognize, and load fails with `commonParamsParse: invalid argument: --tools-mode`. This translation existed in commit 2840f4d but was lost during a later main merge that brought in the new addon constructor shape (PR #1688). Reinstating the original mapping: toolsMode: "dynamic" → tools_compact: "true" toolsMode: "static" → tools_compact: "false"

github-actions · 2026-04-28T04:18:30Z

QVAC E2E — `android` — ⚠️ no results

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts

The test job did not produce a results artifact. Check the run for job-level failures.

github-actions · 2026-04-28T04:18:31Z

QVAC E2E — `ios` — ⚠️ no results

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts

The test job did not produce a results artifact. Check the run for job-level failures.

github-actions · 2026-04-28T04:18:32Z

QVAC E2E — `windows` — ✅ all tests passed (88/88, 676s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts

github-actions · 2026-04-28T04:18:33Z

QVAC E2E — `linux` — ❌ failed

Totals: 87/88 passed · 1 failed · 98.9% · 454s
Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts

Results by section

addon: 1/2 ❌

Failed tests

addon-logging-during-inference: Inference logging error: Cannot set new job: a job is already set or being processed

github-actions · 2026-04-28T04:18:34Z

QVAC E2E — `macos` — ⚠️ no results

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts

The test job did not produce a results artifact. Check the run for job-level failures.

opaninakuffo · 2026-04-28T10:15:42Z

+  promptTokens: z.number().optional(),
+  generatedTokens: z.number().optional(),


generatedTokens & promptTokens arent mapped through from the addon, kindly link

opaninakuffo · 2026-04-28T10:24:32Z

-    if (!canSlice && savedCount > 0) {
+    if (savedCount > 0 && savedCount <= history.length) {
      cachedMessageCounts.delete(cachePathToUse);
    }



Old code deleted stale counts (!canSlice && savedCount > 0 ⇒ savedCount > history.length). New code deletes valid counts (savedCount > 0 && savedCount <= history.length) and leaves stale ones in place. Combined with the fact that savedCount is no longer used for slicing at all, the cachedMessageCounts map has been quietly demoted from "track how much we've cached so we can resend the delta" to "we toggle a bit and then immediately overwrite it via recordCacheSaveCount." Either:

the map is now load-bearing only for the missing-cache-file failure path of recordCacheSaveCount, in which case the predicate should match the old "stale" intent (savedCount > history.length), or

the slicing-by-savedCount behavior should be restored for the cache-hit path, or

the map and recordCacheSaveCount should be removed entirely if they're truly dead.

Kindly document with whichever option as the current state is now contradictory with previous.

opaninakuffo · 2026-04-28T10:32:18Z

Suggestions

from Cursor:

packages/sdk/server/bare/plugins/llamacpp-completion/ops/completion-stream.ts:259-279 — static-mode multi-message turns now send only the last message. Previously slice(savedCount) allowed callers to push multiple messages between completions and have them all forwarded. The new branch structure is lastMessages = [lastMsg] for static mode + non-tool/non-user-with-prev-assistant cases. For the canonical 1-msg-per-turn flow this is fine, but if a consumer (or a recovery path after an error) appends [assistant, user] or [user, user] between completions in static mode, only the last one will reach the model. Worth either calling out as an explicit precondition in the docstring above the function, or restoring the slice-from-savedCount behavior for the static path.
packages/sdk/server/bare/plugins/llamacpp-completion/ops/completion-stream.ts:430-438 — toolsModeForHash is dead/duplicate. Inside the if (kvCache) branch, modelConfig shadows the outer same-named variable (both come from getModelConfig(modelId)), and toolsModeForHash re-derives the same value as the outer toolsMode. Just use toolsMode and drop the inner getModelConfig + toolsModeForHash.
packages/sdk/server/bare/plugins/llamacpp-completion/ops/completion-stream.ts:418-433 — historyWithTools is computed even when kvCache is set and unused on that path. In the kvCache branch we pass raw history into prepareMessagesForCache and let addTools re-append. Move the insertToolsIntoHistory call into the else (no-cache) branch only, so we don't pay for the array spread when it's discarded.
packages/sdk/server/bare/plugins/llamacpp-completion/ops/completion-stream.ts:266-279 — unnecessary as HistoryMsg casts. history is already typed HistoryMsg[], so history[history.length - 1] as HistoryMsg and (history[i] as HistoryMsg) add noise and silence real type errors if the parameter ever loosens. Drop the casts.
packages/sdk/schemas/tools.ts:6-12 — naming for the new const is inconsistent with neighbors. The repo's pattern for value-as-enum is uppercase (VERBOSITY in llamacpp-config.ts, MODEL_TYPES exported from schemas/index.ts) with a separate PascalCase type alongside (ModelType). ToolsModeType as a runtime value with PascalCase + Type suffix reads like a type. Suggest TOOLS_MODE = { static: "static", dynamic: "dynamic" } as const plus type ToolsMode = (typeof TOOLS_MODE)[keyof typeof TOOLS_MODE]. Public API change, but the constant is brand new in this PR so no ecosystem cost.
packages/sdk/server/bare/plugins/llamacpp-completion/plugin.ts:62-66 — tools_compact is now emitted on every llamacpp model load. Because the schema default is toolsMode: "static", transformLlmConfig will set tools_compact: "false" for every model that goes through llmConfigSchema's transform. If the addon treats absent vs. "false" differently (or if some downstream tooling doesn't expect this key), this is a quiet behavior change. Worth confirming with the addon contract and/or only emitting tools_compact when the user explicitly opted into dynamic.
packages/sdk/examples/llamacpp-dynamic-tools.ts — clean up before shipping as a public example:
- runToolInvocationContTest is exported but never invoked, contains commented-out blocks, and duplicates most of runToolInvocationTest. Either delete or rewrite as a focused second example.
- Trailing commented-out invocations (// using same kvCache for a single session, // await runToolInvocationContTest(...)) should go.
- Mixes single and double quotes (toolsMode: 'dynamic' next to role: "system"); the rest of examples/ and the SDK use double quotes.
- tools3 declares parameters: z.object() (no shape arg) — depending on Zod version this either errors at runtime or produces an unintended schema.

DmitryMalishev force-pushed the feature/llm-dynamic-tools branch from 615c9e7 to 1e7b0f7 Compare March 10, 2026 12:44

olyasir force-pushed the feature/llm-dynamic-tools branch from 06b0ae7 to c1e85c2 Compare March 13, 2026 09:19

This was referenced Mar 13, 2026

(feature) llamacpp-llm: dynamic tools #706

Merged

(experiment) llm tools: position before user prompt and after #232

Closed

Base automatically changed from feature/llm-dynamic-tools to main March 21, 2026 09:09

github-code-quality Bot found potential problems Mar 23, 2026

View reviewed changes

(improvement) sdk: dynamic tools integration

5094f21

mialso force-pushed the improvement/sdk-dynamic-tools-interface-support branch from 96e46e2 to 5094f21 Compare March 23, 2026 14:02

mialso marked this pull request as ready for review March 23, 2026 14:03

mialso requested review from a team as code owners March 23, 2026 14:03

mialso changed the title ~~(improvement) sdk: dynamic "toolsMode"~~ (improvement) sdk: "dynamic" tools mode Mar 23, 2026

olyasir previously approved these changes Mar 23, 2026

View reviewed changes

mialso changed the title ~~(improvement) sdk: "dynamic" tools mode~~ QVAC-13559 feat: sdk "dynamic" tools mode Mar 23, 2026

mialso changed the title ~~QVAC-13559 feat: sdk "dynamic" tools mode~~ QVAC-13559 feat[api]: sdk "dynamic" tools mode Mar 23, 2026

opaninakuffo reviewed Mar 23, 2026

View reviewed changes

Comment thread packages/sdk/server/bare/plugins/llamacpp-completion/ops/completion-stream.ts Outdated

opaninakuffo reviewed Mar 23, 2026

View reviewed changes

(chore) sdk: rename history append, improve tests

5f5ce14

mialso dismissed olyasir’s stale review via 5f5ce14 March 23, 2026 23:09

mialso added 2 commits March 24, 2026 05:28

Merge remote-tracking branch 'origin/main' into improvement/sdk-dynam…

a26efdd

…ic-tools-interface-support

Merge branch 'main' into improvement/sdk-dynamic-tools-interface-support

a5f3d06

NamelsKing reviewed Mar 24, 2026

View reviewed changes

Comment thread packages/sdk/examples/llamacpp-dynamic-tools.ts Outdated

(chore) sdk: update bun.lock, example sdk import

2a10f45

DmitryMalishev added the verify label Mar 24, 2026

Merge remote-tracking branch 'origin/main' into improvement/sdk-dynam…

0d3a650

…ic-tools-interface-support

olyasir previously approved these changes Mar 25, 2026

View reviewed changes

simon-iribarren reviewed Mar 25, 2026

View reviewed changes

mialso and others added 7 commits April 24, 2026 17:38

(internal) sdk: debug stats handling

a55fc18

Merge remote-tracking branch 'origin/main' into improvement/sdk-dynam…

a489553

…ic-tools-interface-support

(chore) sdk: native tools example no cache

06d5e0f

(internal) sdk: add bun-lock, native tools without dynamic

1667ce0

Merge branch 'main' into improvement/sdk-dynamic-tools-interface-support

514f13a

Merge remote-tracking branch 'upstream/main' into QVAC-13559-sdk-dyna…

710fb87

…mic-tools

This comment has been minimized.

Sign in to view

github-code-quality Bot found potential problems Apr 28, 2026

View reviewed changes

Comment thread packages/sdk/tests-qvac/tests/shared/executors/tools-executor.ts

stream?: boolean;

};

const toolsModelId = await this.resources.ensureLoaded("tools");

const resourceDep = p.toolsMode === ToolsModeType.dynamic ? "tools-dynamic" : "tools"

This comment has been minimized.

Sign in to view

opaninakuffo reviewed Apr 28, 2026

View reviewed changes

RamazTs mentioned this pull request Apr 28, 2026

QVAC-13559 feat[api]: sdk "dynamic" tools mode #1779

Merged

lauripiisang mentioned this pull request Apr 28, 2026

infra[notask]: fix sdk e2e prior-comment minimize and missing ios/android report summary #1780

Merged

		promptTokens: z.number().optional(),
		generatedTokens: z.number().optional(),

Conversation

mialso commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes:

how to check it works:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tier-based Approval Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mialso Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NamelsKing commented Mar 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mialso Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QVAC E2E — android — ⚠️ no results

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QVAC E2E — ios — ⚠️ no results

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QVAC E2E — windows — ✅ all tests passed (88/88, 676s)

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QVAC E2E — linux — ❌ failed

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QVAC E2E — macos — ⚠️ no results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

opaninakuffo commented Apr 28, 2026

Suggestions

Uh oh!

Reviewers

mialso commented Mar 6, 2026 •

edited

Loading

github-actions Bot commented Mar 23, 2026 •

edited

Loading

mialso Mar 23, 2026 •

edited

Loading

mialso Mar 25, 2026 •

edited

Loading

github-actions Bot commented Apr 28, 2026 •

edited

Loading

QVAC E2E — `android` — ⚠️ no results

github-actions Bot commented Apr 28, 2026 •

edited

Loading

QVAC E2E — `ios` — ⚠️ no results

github-actions Bot commented Apr 28, 2026 •

edited

Loading

QVAC E2E — `windows` — ✅ all tests passed (88/88, 676s)

github-actions Bot commented Apr 28, 2026 •

edited

Loading

QVAC E2E — `linux` — ❌ failed

github-actions Bot commented Apr 28, 2026 •

edited

Loading

QVAC E2E — `macos` — ⚠️ no results