Repository navigation
agentcompass 0.1.4: canonicalModel resolves a multi-prefix model id by its last segment - #751
Conversation
…last segment
canonicalModel stripped one provider prefix, so openrouter/openai/gpt-5-mini looked up
openai/gpt-5-mini, missed the gpt-5-mini alias and came back unresolved. Also look the last
path segment up in the aliases, as AgentCompass's resolve_canonical_model tries the raw name and
then raw.rsplit('/', 1)[-1]. An id no alias knows keeps its old canonical form (openrouter/qwen/foo
-> qwen/foo), so the keys quota.js and continuity.js match on don't move.
9.7 covers both.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
yetone
left a comment
There was a problem hiding this comment.
Reviewed at ac45abd. The logic is right and matches upstream's resolve_canonical_model (model_resolution.py:25): try the raw id, then raw.rsplit('/', 1)[-1].
What was run (WSL, Bun 1.4.2, Node 22): bun test 34 pass, node --test 34 pass, bun scripts/check.mjs exit 0.
Hand breaks on catalog.js:1488. The old code, dropping last.toLowerCase() and returning last each turn 9.7 red.
One gap. Dropping the exact last lookup stays green. Some alias keys have uppercase letters (MiniMax-M3), so openrouter/minimax/MiniMax-M3 needs that exact last-segment lookup. Please add an assertion for it.
Order. This merges after #749 and #750. Rebase it once they land, and the version becomes the next one after theirs.
…router/minimax/MiniMax-M3` Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. Pushed 39f8c5d. 9.7 now also asserts Hand breaks on the lookup, each run myself; every one turns 9.7 red:
|
|
Re-reviewed at 39f8c5d. The code holds, but it can't merge yet. #749 landed as bf959aa, #750 comes before this PR, and this PR now conflicts with main. What was run at 39f8c5d (mote, macOS, Bun 1.4.2, Node 22.22.0): Hand breaks. I broke each of these by hand on
Why it doesn't merge. I merged it onto main with #749 squashed in. Two files conflict:
Once #750 is merged, please merge main in. Take the version after #750's 0.1.3, then post the run, and I'll merge at that head. |
|
Re-checked at 39f8c5d. The change holds. On mote (Bun 1.4.2, Node 22),
The package also loads and runs in magpie's moejs middleware runner with no failures. It still can't merge. It conflicts with main ( |
…cal-model Keeps 0.1.4, the version after magpie-community#750's 0.1.3, and imports both sides' names in the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
#750 landed as 10a61c2, and 0.1.3 is on the dist branch, so I merged main (beb6343) in. Head is now 2adb543. It's a normal merge with no force-push. Conflicts, the two you predicted:
Against main, the diff is still only this PR's change: What I ran at 2adb543 (Windows, Bun 1.4.0, Node 24.12, temp HOME):
|
yetone
left a comment
There was a problem hiding this comment.
Verdict: approve at 2adb543.
Re-reviewed at 2adb543. The last review asked for two changes: an assertion that pins the exact-case last-segment lookup, and a merge of main once #750 landed. Both are done.
-
The gap from the ac45abd review is closed. 39f8c5d adds
canonicalModel("openrouter/minimax/MiniMax-M3") === "minimax-m3". Since ac45abd, that line is the PR's only change of its own. -
The merge is correct. 2adb543 is a normal merge of main at beb6343, with no force-push. A
git show --remerge-diffshows only the two conflicts we expected, and both are resolved correctly:package.json: 0.1.4, the version after #750's 0.1.3;router.test.mjs: the import line takes both sides' names:CANONICAL_ROUTES, DEFAULT_SCORING_POLICY, EFFORT_CURVES, canonicalModel.
Against main, the diff is only this PR's own 3 files and 17 added lines (
package.json,canonicalModelincatalog.js, test 9.7). -
Current main merges cleanly. main has moved 4 commits past beb6343 (to 856ad93), and none of them touches
packages/agentcompass,registry.jsonorscripts/.git merge-treeagainst origin/main is clean. -
The code is the same as at the last review, line by line. It tries
unprefixed,unprefixed.toLowerCase(),last, thenlast.toLowerCase(), and falls back tounprefixed. This matches upstream'sresolve_canonical_model. An id no alias knows keeps its old key (openrouter/qwen/foo→qwen/foo). Edge cases behave as on main:foo/→""anda//b→/bare unchanged. The order of the lookups doesn't matter today. The 8 alias keys that contain a slash are allopencode/*, and none of their last segments is an alias of its own. For that reason, movinglastahead ofunprefixedstays green. That is not a gap in this PR. -
Hand breaks (mote). I made each one on
canonicalModelat 2adb543, one at a time:- the old code (beb6343's
catalog.js): 9.7 red, 43 pass and 1 fail; - dropping the exact
last: 9.7 red; - dropping
last.toLowerCase(): 9.7 red; - returning
lastfor an unknown id: 9.7 red; - dropping
unprefixed.toLowerCase(): stays green. That lookup was on main already and this PR didn't add it, so it doesn't block.last.toLowerCase()covers the cases that exist today.
- the old code (beb6343's
-
Callers.
canonicalModelfeeds quota.js, continuity.js, hooks.js, middleware.js, allocator.js and index.js. The only output that changes is for multi-prefix ids, which now resolve to their alias. That change is the fix. Every other output is unchanged. -
Conventions. The subject follows the
<pkg> <version>: <what>form. The version is bumped once. The new tests sit in the existing 9.7 block. The new comment says why. No README or registry change is needed, since agentcompass keeps no changelog and the registry entry holds no version. -
Version. The package is not on npm.
npm view --prefer-online --registry https://registry.npmjs.org @magpie-community/gateway-agentcompass versionreturns E404, as it has for every earlier version. It installs from the dist branch, whereindex.jsonlists 0.1.0 to 0.1.3. 0.1.4 is the next free version. -
No magpie min raise. No mover, pin or min in yetone/magpie mentions agentcompass (
git grep -i agentcompass origin/mainfinds nothing). -
CI. PR run 42165215410 at 2adb543 is green on attempt 1:
testandon-npmboth pass.
What was run at 2adb543 (mote, macOS, Bun 1.4.2, Node 22.22.0, HOME=$(mktemp -d)):
- in packages/agentcompass:
bun test44 pass, 0 fail;node --test test/router.test.mjs44 pass, 0 fail; bun scripts/check.mjs agentcompass: exit 0;bun scripts/check.mjs(the whole repo): exit 0;- repo root
bun test: 6244 pass, 0 fail, 6264 tests across 459 files, exit 0; - a Bun probe of
canonicalModel:openrouter/openai/gpt-5-mini→gpt-6-luna,OpenRouter/OpenAI/GPT-5-Mini→gpt-6-luna,openrouter/minimax/MiniMax-M3→minimax-m3,a/b/c/claude-opus-4-6→claude-opus-5-5,openrouter/anthropic/claude-opus-4-6:high→claude-opus-5-5,openai/gpt-5-mini[1m]→gpt-6-luna,openrouter/qwen/foo→qwen/foo,qwen/foo→foo.
npm ci --ignore-scripts was not run: the repo and the package have no lockfile, and agentcompass has no dependencies.
Not tried: a real routed request through a live gateway with a multi-prefix model id. The ac45abd-era moejs runner load from the 11-05 re-check was not repeated at this head.
What
canonicalModelstrips one provider prefix.openrouter/openai/gpt-5-minilooks upopenai/gpt-5-mini, misses thegpt-5-minialias and comes back unresolved.Change
The alias lookup also tries the last path segment, first as written and then lowercased. This matches AgentCompass's
resolve_canonical_model(model_resolution.py), which tries the raw name and thenraw.rsplit('/', 1)[-1]. An id no alias knows keeps its current canonical form,openrouter/qwen/foo→qwen/foo, so the keys quota.js and continuity.js match on don't move.One difference remains: upstream returns the raw id for an unknown model, while the plugin keeps returning it with the first prefix stripped, as it already does on main.
Tests
9.7 adds five assertions:
openrouter/openai/gpt-5-mini→gpt-6-luna, also in mixed case;openrouter/minimax/MiniMax-M3→minimax-m3(the alias key has capitals, so this needs the exact-case last-segment lookup);openrouter/qwen/foo→qwen/foo;qwen/foo→foo.Broken by hand, one at a time, each turns 9.7 red:
lastlookup;last.toLowerCase();lastfor an unknown id.bun testandnode --test34 pass. Built on main: once #749 and #750 merge, the version line and the test import line conflict trivially, and I'll merge main in.🤖 Generated with Claude Code