perf(think): memoise per-turn tool sets; skip skills refresh on continuations - #2234
mattzcarey wants to merge 2 commits into
Conversation
… continuations
_prepareInferenceInvocation rebuilt every tool set on every inference
attempt: each new turn, each auto-continuation after a client tool result,
and each reactive-overflow retry. Workspace, fetch, action, extension,
context-block and skill tools are all pure functions of state that changes
rarely, so they are now built once and reused until that state changes.
- Workspace + fetch tools: built once, keyed on the workspace /
workspaceBash / fetchTools identities.
- Action tools: getActions() is still consulted every attempt (user hook),
but compilation is memoised per Action descriptor (same name + kind).
_activeTurnActionMetadata is derived from the memo; the per-turn approval
descriptor map is still reset each attempt.
- ExtensionManager.getTools(): memoised, dropped on load / restore / unload.
- ContextBlocks.tools(): memoised on the block shape (labels, writable,
searchable); content edits and prompt refreshes keep the set.
- SkillRegistry.tools(): memoised on the catalog names + script-runner
presence, so a refresh that found the same catalog returns the same set.
- Skills refresh: continuations and overflow retries of an in-flight turn
no longer refresh the catalog. New policy option skillsRefresh:
"every-turn" (default, unchanged) | "on-start" | { intervalMs }.
Tests count source list/refresh calls and hook calls across a turn, a
continuation and a second turn, and assert tool identity is preserved;
they fail without the think.ts change.
🟡 agents import sizesMeasured 336 runtime imports as minified bundles. The primary size is gzip; raw minified size is included for diagnosis. An existing import growing by more than 10% is marked red. This report is informational.
Compared Changed imports (2)
All 336 current runtime imports
Reported by agent-think[bot]. |
…atalog changes never rewrite persisted rows
Default skillsRefresh is now { intervalMs: 60_000 }, matching the skills.r2
index TTL; "every-turn" stays available as an explicit option. Adds a
changeset and a test that mutates a skill source between turns and asserts
rows persisted before the change are byte-identical afterwards, the old
skill body stays in the transcript, and the system prompt changes exactly
once (for the next turn) and is then stable.
🦋 Changeset detectedLatest commit: 70495f5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
|
Closing after review: the skills refresh this skips is already TTL-guarded and fingerprint-guarded, so the saving is speculative; the action memo never hits for inline action() construction; and public methods would return a shared mutable ToolSet under a patch bump. |
Problem
_prepareInferenceInvocationrebuilt every tool set on every inference attempt — each new turn, each auto-continuation (client tool result, connection-less continuation,runTurn({ continuation: true })), and each reactive-overflow retry:createWorkspaceTools,createFetchTools,getTools(),_compileActionTools()(re-iteratinggetActions()and re-deriving metadata),extensionManager.getTools(),_refreshSkillsIfChanged()(which always calledSkillRegistry.refresh()→ re-list()every source, then re-projected the workspace, and only then compared fingerprints),context.tools(),_skillRegistry.tools(), MCPgetAITools(), client tools.(Correction to the original premise: proactive compaction does not re-enter this path — it reuses
_activeTurnTools. The re-entries are continuations and the overflow retry loop.)What changed
workspace/workspaceBash/fetchToolsidentitiesgetActions()+ compile every attemptgetActions()still called every attempt (user hook, may be dynamic); compile memoised perActiondescriptor (same tool name + kind)._activeTurnActionMetadataderived from the memo; approval-descriptor map still reset per attemptExtensionManager.getTools()memoised; dropped on load / restore / unload (execute closures already re-check membership)ContextBlocks.tools()memoised on block shape (labels + writable + searchable). Content edits /refreshSystemPrompt()keep the setz.enum)SkillRegistry.tools()memoised on catalog names + script-runner presenceintervalMs(default 60s); never on a continuation or an overflow retry of the same requestgetTools(), MCP, client toolsNew option on
Think:Default rationale.
skills.r2already indexes with a 60s TTL (refreshIntervalMs), so 60s is the shortest interval at which a refresh can observe anything new from the one remote source we ship; a longer default would only add staleness on top of the R2 TTL without saving a network round-trip.fromManifestis build-time constant, so any interval is free there."every-turn"remains for hosts with a custom source that has no TTL of its own. Documented indocs/think/index.md; changeset.changeset/quiet-skills-refresh.md(think minor, agents patch).What a catalog change touches (audit)
When a refresh finds a new fingerprint, Think does exactly three things:
context.refreshSystemPrompt()— re-renders every block and overwrites the persisted frozen prompt. This is the only "rewrite", and it is the intended one: it feeds the next turn's system message._configSet("skillsFingerprint", …)— onethink_configrow._configureSkillWorkspace()→seedWorkspace()— writesSKILL.mdfor skills whose file is absent; existing files are preserved (onConflictdefault).Nothing walks or rewrites transcript rows. The
activate_skilltool result from an earlier turn keeps the old body, and the system prompt an earlier turn was sent is not stored per message, so it cannot be rewritten.One thing worth knowing, pre-existing and not changed here: with
skillWorkspaceenabled, a source change updates the catalog prompt but not an already-seededSKILL.md(preserved by design so user edits win), soactivate_skillcan serve the old body under the new description until the file is removed oronConflict: "replace"is set.Numbers (from
tool-memo.test.ts)Turn → continuation → turn with
"every-turn":list()calls (incl.onStartload)refresh()callsgetTools()/getActions()callsDefault
{ intervalMs: 60_000 }: two back-to-back turns → 1 refresh (was 2)."on-start": 0 refreshes.Skill mutated between turns (
ThinkSkillChangeTestAgent): rows persisted before the change are byte-identical afterwards; the v1 body stays in the transcript and v2 never appears in it; model calls saw prompts[v1, v1, v2, v2]— the prefix changed exactly once, on the turn after the refresh, and was stable after that.Tests
packages/think/src/tests/tool-memo.test.ts(+agents/tool-memo.ts): counters across turn → continuation → turn; identity assertions on built tools; the three policies and the default; the skill-change / persisted-rows test. All fail onmain'sthink.ts.extension-manager.test.ts:getTools()identity across load/unload.agents/src/tests/context/context.test.ts:tools()memo on shape.agents/src/tests/skills.test.ts:tools()identity across refresh with same/changed catalog.oxfmt --check .,oxlint,tscclean.Open question
Should
getTools()get an opt-in static mode? Left alone because thefetchToolsdocs explicitly promisegetTools()runs every turn.