handles instead of ref-ids - #1533
Open
gonzaloaune wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cut
create_batch_triplettoken cost: node handles + trimmed resultsWhy
create_batch_tripletis 56.7% of all agent wall-clock time in production, and itssteps are generation-bound — the cost is the model writing JSON, not executing it
(Neo4j sits near 0% CPU during these steps, after the separate query fixes in
jarvis-backend).
Four full production conversation traces were tokenized (
cl100k_base):Arguments to one tool are 78% of everything the model generates. Two independent
problems inside that, each addressed here.
1. ref_ids are 36.6% of argument tokens
A UUID costs 23 tokens every time the model writes one. Across the four traces:
2. Results echo back what the caller just sent
Results average 4,349 tokens per call — larger than the arguments — and each entry
repeated three fields the caller already had:
{"status":"Success", "source_ref_id":"691c3551-…", // caller supplied "target_ref_id":"a0f45762-…", // caller supplied "edge_ref_id":"020cfe85-…", // never used "edge_type":"CONTAINS"} // caller suppliedMeasured across all four traces: 1,426
edge_ref_idvalues returned, 0 everreferenced in a later call. Node ref_ids are the opposite — 221 of 222 get reused,
because that is how the agent learns the ids of nodes it created inline.
What changed
Node handles (
@nN)A per-run table maps
ref_id ↔ handle. Every tool that surfaces a node now returns@n7in place of the raw ref_id; every tool that accepts a ref_id takes either form.Jarvis only ever sees real ref_ids — resolution happens at the tool boundary.
"@n7"and"@n148"both tokenize to 4 tokens, against 23 for a UUID. The@prefix is free and makes a handle unmistakable in a payload.nameandnode_type,so the model still knows what
@n7is without a legend.graph_search,graph_get,graph_neighborsgraph_get,graph_neighborscreate_triplet,create_batch_tripletcreate_triplet,create_batch_tripletreturn_edge_ids(opt-in, default off)Successful batch entries no longer carry
edge_ref_id(opt-in via the new flag) oredge_type(a pure echo, always dropped). Error entries keep both — the context isworth the tokens and the volume is negligible.
Guards
findHandleInNodeDatarejects a handle used as a property value on either side ofeither write tool. A handle persisted as node data would be untraceable garbage.
graph_sub_agentrewrites@nNto real ref_ids in the prompt before spawning: thechild builds its own table, so the parent's handles mean nothing to it.
create_batch_tripletthis fails that item only, matching the existingpartial-failure contract.
Expected impact
edge_ref_id+edge_typeResults persist in context and are re-sent on every subsequent step (context grew
6,820 → 180,494 over 26 steps in one traced session, 2.40M cumulative input), so the
result-side savings compound.
Backward compatibility
resolve()passes through anything thatis not
@n\d+, so ids from session history, a sub-agent prompt, or another processkeep working.
return_edge_idsdefaults tofalse; set it to restore the previous field.Known gap — read before merging
Persisted sessions will contain
@nNin stored tool calls. These resolve correctlyin-process, but a session resumed in a different process — or replayed via
transparentmode — starts with an empty table, and the model could reuse a stale handle from
replayed history.
The fix is to rewrite handles back to real ref_ids at persist time in
extractMessagesFromSteps(utils.ts), plumbing ahandleTableRefthroughget_toolsthe way
messagesRefandprovenanceCollectoralready are. Not in this PR. Ifmulti-turn or replayed sessions are in scope for this deploy, that should land first.
Scope was verified for the in-process case:
get_toolsis called fromprepareAgent,which both entry points (
get_context,stream_context) invoke fresh per request withno memoization, so the closure is correctly per-run. There is a test asserting two
tables are independent — it looks trivial but pins the one assumption that would
silently resolve one document's handle into another document's node if
get_toolseverbecame memoized.
Tests
New in
mcp/src/repo/__tests__/toolsJarvis.test.ts:createHandleTable— mint idempotence, sequential allocation, resolve, UUIDpass-through, near-miss rejection (
@node7,n7), unknown-handle error, rewritesemantics, table independence
accepted, sub-agent prompt rewrite
findHandleInNodeData— rejects handle-as-value, allows"see @note7", trims firstreturn_edge_ids— off by default, on when set, changes nothing elsesimulateBatch(the in-test mirror of the production result assembly) was updated tomatch, including the new flag, so it does not drift.