Skip to content

perf(tools): stop embedding the layer list in every map-tool description (#225) - #271

Merged
cboettig merged 1 commit into
mainfrom
perf/tool-desc-drop-layer-list-225
Jun 27, 2026
Merged

perf(tools): stop embedding the layer list in every map-tool description (#225)#271
cboettig merged 1 commit into
mainfrom
perf/tool-desc-drop-layer-list-225

Conversation

@cboettig

Copy link
Copy Markdown
Member

Closes #225.

What

Each layer-targeting map tool (show_layer, hide_layer, set_filter, clear_filter, reset_filter, set_tooltip, reset_tooltip, set_style, reset_style, filter_by_query10 sites) re-embedded the full layer roster via formatLayerList(allLayers()/vectorLayers()). This drops those appends and the now-dead allLayers / vectorLayers / formatLayerList helpers.

Note: the issue estimated 14 sites incl. move_layer_* / add_hex_tile_layer. In current main those tools don't embed the list — actual count is 10.

Why it's safe (loses no information)

The layer roster is already injected once into the system prompt by DatasetCatalog.generatePromptCatalog() — and richer: layer_id + title + type + versions + default filters, vs. the per-tool id — displayName subset. Both sides key on the same composite ${ds.id}/${assetId} ID (dataset-catalog.js:727 == getLayerSummaries() keys), so there was no ID-format divergence — just redundancy.

  • Browser path: main.js:339-340 appends generatePromptCatalog() to the system prompt.
  • Headless runner (open-llm-proxy/headless): same — its system prompt is system-prompt.md + generatePromptCatalog() + MCP prompt.
  • Live truth remains available to the agent via get_map_state.

The per-tool copies were also frozen at boot (descriptions built once in createMapTools), so runtime-added hex-… layers never appeared in them anyway — dropping them removes a stale surface.

pickLayerNudge is kept per-tool — that's the disambiguation rule, not the list (per the issue's non-goals).

Test

Replaced the set_tooltip description lists vector layers only assertion with one that locks in the new contract across 5 tools (no embedded list, no Available layers: / Vector layers: headers) plus a check that the nudge is retained. Full suite: 344 passing.

Validation plan (before fleet pin)

Apps pin stable releases, so merging here doesn't propagate. Before cutting a release, A/B this branch vs main across the open-model collection via the open-llm-proxy headless matrix (prefill tokens + tool-call correctness). That requires a small GEO_AGENT_BRANCH param in the matrix Job — separate PR in open-llm-proxy.

…ion (#225)

Each layer-targeting map tool re-embedded the full layer roster via
formatLayerList(), frozen at boot. That was 10 copies of a strict subset
of the list DatasetCatalog.generatePromptCatalog() already injects once
into the system prompt (which carries id + title + type + versions +
default filters — strictly richer). Same composite `${ds.id}/${assetId}`
IDs on both sides, so the per-tool copies added no information — just
~N×30-50 chars × 10 tools of redundant, boot-frozen text on every call,
and extra tool-description surface that nudges wrong-tool selection.

Drop the per-tool appends and the now-dead allLayers/vectorLayers/
formatLayerList helpers. Keep pickLayerNudge — that's the per-tool
disambiguation rule, not the list. The roster still reaches the agent
via the system-prompt catalog (browser: main.js; headless runner:
generatePromptCatalog in its system prompt), and live truth is available
via get_map_state.

Test: replace the 'lists vector layers' assertion with one that locks in
the new contract (no embedded list; nudge retained).
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 51.38% 3756 / 7310
🔵 Statements 51.36% 3756 / 7313
🔵 Functions 58.63% 129 / 220
🔵 Branches 82.58% 754 / 913
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
app/map-tools.js 96.19% 87.93% 50% 96.19% 88-90, 105-107, 423-437
Generated in workflow #118 for commit 352b73e by the Vitest Coverage Report Action

@cboettig

cboettig commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

🔬 Eval workflow & merge ordering (pickup notes)

This PR should not merge on faith — validate it on the open model collection first, via the headless matrix. Steps, in order:

1. Merge the lever first. boettiger-lab/open-llm-proxy#43 adds GEO_AGENT_BRANCH to the matrix Job. Until it's on main, the matrix always clones geo-agent main and can't see this branch. So: merge #43 → then run the A/B below → then decide on this PR.

2. Run the A/B (from open-llm-proxy/headless, same questions file both arms):

cat > runs/q.txt <<'Q'
show me protected areas in California
filter to GAP status 1
hide the carbon layer
Q

# baseline = current main
TAG=baseline GEO_AGENT_BRANCH=main \
  QUESTIONS_FILE=runs/q.txt ./run-matrix-k8s.sh boettiger-lab/geo-agent-template
# fix = this branch
TAG=tooldesc GEO_AGENT_BRANCH=perf/tool-desc-drop-layer-list-225 \
  QUESTIONS_FILE=runs/q.txt ./run-matrix-k8s.sh boettiger-lab/geo-agent-template

3. Compare (after ./sync-logs.sh). ORIGIN suffix follows the TAG (agent_runner_<tag>):

SELECT
  regexp_extract(origin, 'agent_runner_(\w+)', 1)               AS arm,
  count(*)                                                      AS llm_calls,
  round(avg(json_extract(tokens,'$.prompt_tokens')::BIGINT))     AS avg_prompt_tokens,
  round(avg(json_extract(tokens,'$.completion_tokens')::BIGINT)) AS avg_completion_tokens,
  sum(len(tool_calls))                                          AS total_tool_calls
FROM read_ndjson_auto('/tmp/open-llm-proxy-logs/*/*.jsonl', union_by_name=true)
WHERE type='response'
  AND (origin LIKE '%agent_runner_baseline' OR origin LIKE '%agent_runner_tooldesc')
GROUP BY 1 ORDER BY 1;

4. Decision gate. Merge this PR if the tooldesc arm shows lower avg_prompt_tokens (the expected win) with tool-call behavior neutral-or-better (no rise in total/wrong-tool calls, no new flailing in the transcripts). If a model regresses on tool selection, hold and dig into that model's transcripts before merging.

Apps pin stable releases, so merging here is safe in isolation — the fleet doesn't move until a release is cut and geo-agent-ops bumps pins. This eval is the gate before cutting that release.

@cboettig

Copy link
Copy Markdown
Member Author

✅ A/B eval results (open models, via open-llm-proxy headless matrix)

Ran the A/B from the "Eval workflow & merge ordering" plan: GEO_AGENT_BRANCH=main (baseline) vs GEO_AGENT_BRANCH=perf/tool-desc-drop-layer-list-225 (this PR), same questions, same models, through the open-llm-proxy k8s matrix. The GEO_AGENT_BRANCH lever (open-llm-proxy#43) is merged, which made this possible. Confirmed the arms differed only in the geo-agent framework checkout (baseline 90d02eb vs this branch 352b73e).

Verdict: passes the gate — recommend merge.

1. Prefill tokens — the expected win ✅

First-turn prefill (system prompt + tool defs + first user msg) dropped ~24% (~9.5k tokens), near-identical across every model with data — exactly the signature of a fixed-size layer roster that was embedded across 10 tool descriptions and is now sent once via the system-prompt catalog:

model baseline tooldesc Δ
glm-5 37,992 28,852 −9,140
gpt-oss 36,414 27,644 −8,770
nemotron 40,690 30,958 −9,732
qwen3 40,002 30,268 −9,734

This is paid every turn, so the saving compounds over a multi-turn session.

2. Tool-call behavior — neutral-or-better ✅

Within the cleanly-completed cells: same correct tool for each task in both arms (set_filter, hide_layer, get_schemashow_layerset_filter), no new flailing in the fix arm, and one fix-arm gpt-oss session correctly drove the full hex pipeline (register_hex_tilesget_hex_tile_statusadd_hex_tile_layer) — directly refuting the worry that dropping the embedded list breaks layer-targeting. (Those runtime hex-… layers were never in the per-tool embed anyway; it was frozen at boot.)

Caveats (honest)

  • The run coincided with an NRP ellm 502/500 incident (symmetric across both arms — ~24–29 5xx each, plus gemma/nemotron timeouts), which thinned the matched-cell sample. So the correctness axis is suggestive, not statistically tight.
  • gemma could not be assessed — it timed out on essentially every cell in both arms (infra, not branch).

The prefill win is unambiguous and large; the correctness signal is neutral-or-better everywhere it could be measured. Reproduction details + queries live in open-llm-proxy/headless (origin tags agent_runner_baseline / agent_runner_tooldesc).

@cboettig
cboettig merged commit 8f59692 into main Jun 27, 2026
1 check passed
@cboettig
cboettig deleted the perf/tool-desc-drop-layer-list-225 branch June 27, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool-description bloat: every map tool embeds the full layer list

1 participant