Repository navigation
Conversation
compressToolDescriptions() calls compressSchemaDescriptions(input_schema, null)
for the schema ROOT, where there is no property name to judge. The first
root-level `description` string therefore reached isObviousFromName(null),
which threw:
TypeError: Cannot read properties of null (reading 'toLowerCase')
at isObviousFromName (src/prompts/system.js:160)
at compressSchemaDescriptions (src/prompts/system.js:115)
The orchestrator catches that throw and logs it as
"System prompt optimization failed, continuing with original"
(src/orchestrator/index.js), so the failure was silent and shipped: with the
default TOOL_DESCRIPTIONS=minimal, minimal-mode tool-description compression did
nothing for every tool whose input schema carried a root-level description. The
sibling optimizeSystemPrompt() call sits in the same try block, so it was
skipped for those requests too.
Treat a non-string key as "not obvious" rather than dereferencing it.
Regression coverage added to test/tool-schema-compression.test.js for a
root-level schema description on object, bare-object and array schema roots.
Contributor
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s). |
This branch has not been deployed
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.
Description
With the default
TOOL_DESCRIPTIONS=minimal, tool-schema compression silently does nothing for any request containing a tool that carries adescriptionon its schema root.compressToolDescriptions()callscompressSchemaDescriptions(input_schema, null)for the root, and the first root-level description hands thatnulltoisObviousFromName(), which callsname.toLowerCase()unguarded — throwingTypeError: Cannot read properties of null (reading 'toLowerCase'). The orchestrator catches the throw and downgrades it to a warning, so the request still returns200with every tool's descriptions left uncompressed. Because the call sits inside a.map()over the tool array, one offending tool disables compression for the entire request. This change treats a non-string key as "not obvious", which is the correct answer for a schema root.Reported from a self-hosted deployment; no affiliation with Lynkr or with any provider.
Closes #121
Problem
Observed on a self-hosted gateway (v9.14.17, OpenRouter provider) on every request that carried tools:
Reproduction.
POST /v1/chat/completionswith a single tool whoseparametersobject carries a top-leveldescription. Minimal mode is the default (src/config/index.js:248), so no configuration change is required.Measured impact. On that gateway, 29 of 126 completed requests (23%) logged the warning, and the share of tool-bearing requests is higher still since the path only runs when tools are present. Each affected request forfeited the whole compression pass — and, because
optimizeSystemPrompt()sits in the sametryblock, that too.Not a duplicate.
mainis at1243a46andisObviousFromName()is still unguarded there; no branch in the repo carries a guard, and no other issue or PR covers this defect.Type of Change
Changes Made
src/prompts/system.js—isObviousFromName()returnsfalsefor any non-string key and its@paramnow documentsnull, so a schema root (compressed withkey === null) is treated as "not obvious" instead of dereferencingnull.test/tool-schema-compression.test.js— appends a suite covering a root-level schema description on object, bare-object and array schema roots. No new file, and the file is already listed inpackage.json'stest:unit, so nopackage.jsonchange is needed.Behavior changes for review
optimizeSystemPrompt()call in the sametryblock now runs as well. That follows directly from removing the throw; it is not a separate change.usage.prompt_tokensagainst an unpatched build.isObviousFromName()simply becomes total over its input domain.Testing
node --test test/tool-schema-compression.test.js(9 pass / 0 fail after; 5 pass / 4 fail before). Also ran the target file plus 13 adjacent suites (14 files, 180 tests): 179 pass, 1 fail — see Known issue.test/tool-schema-compression.test.js(extended; no new file).src/; the same 10-tool payload posted to both; comparedusage.prompt_tokensreturned by the upstream (OpenRouter,z-ai/glm-5.3-flash) and the compression debug line. No real API calls are made in the committed tests.Verified on Node 22.23.3 (the version ci.yml pins) and Node 24.21.0 — identical results.
Measured results
Unit —
node --test test/tool-schema-compression.test.js:All four failures were the newly added tests, failing with the production error (
TypeError ... 'toLowerCase'atisObviousFromName,src/prompts/system.js:160).Billed tokens — 10-tool payload,
usage.prompt_tokensread off the upstream response; deterministic across 3 repetitions per row:The control row is the attribution check: with nothing to trip the bug, both builds are indistinguishable. Re-running the first two rows with
SYSTEM_PROMPT_MODE=static(which takesoptimizeSystemPrompt()out of play) reproduces them exactly, so the saving is attributable to tool-schema compression alone.Worth knowing when reading the debug log: its char-based figure overstates the win — it reports 63.3% of characters saved against 52.4% of billed tokens, because what gets dropped is prose.
Lint —
npx eslint@8.57.0 src/prompts/system.js --max-warnings 0(exit 0).Known issue (follow-up, not addressed here)
test/tool-schema-compression.test.js..github/workflows/ci.ymlruns its own hardcodednode --testlist of 19 files, whilepackage.json'stest:unitlists 99 — 80 files never execute on a PR. The file holding this regression suite (and the existing Tool schema compression rebuildsinput_schemafrom a short allowlist and drops strict, pattern, const and oneOf #116 tests) is one of them. Adding it to the workflow list is a one-line change, deliberately left out here to keep this PR scoped.test/context-compression.test.jsalready fails on unmodifiedmainwithReferenceError: describe is not defined— it never imports fromnode:test— and is not in the CI list. Pre-existing; untouched here.main:npx eslint src index.js --max-warnings 0fails onsrc/clients/cursor-utils.js:34('execSync' is assigned a value but never used), introduced by feat: Add support for Cursor Pro #119. Unrelated to this change — the file touched here lints clean — and open PR Routing hygiene for agent harnesses, structured-output fidelity, OpenRouter hardening, shortfall semantic signals #120 appears to remove that unused import.input_schemafrom a short allowlist and drops strict, pattern, const and oneOf #116 is a different defect and remains open: the feat:Tier routing on subscription passthrough, Jev routing judge, ups… #117 rewrite losesstrict/pattern/const/oneOffrom schemas. This PR does not address it.Checklist
npx eslint@8.57.0 src/prompts/system.js --max-warnings 0exits 0 (the repo-widesrc index.jsinvocation fails only on the pre-existing error above)