Repository navigation
test: consolidate Z.AI GLM-5.3 registry catalog tests into a golden table - #2326
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description follows the required template. It explains the change and rationale, documents impact, lists testing commands and results, identifies skipped checks, and records the tested provider/mo… |
| Title check | |
The title is concise, scoped, and accurately describes the consolidation of the Z.AI GLM-5.3 registry catalog tests shown in the diff. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Risk Surface Disclosed | |
PASS. The PR changes only src/integrations/runtimeMetadata.test.ts; no production auth, routing, permissions, network, startup, plugin, CI, or release code changes. The PR description identifies the… |
| No Hidden Policy Change | |
PASS. The pull request changes only src/integrations/runtimeMetadata.test.ts. The diff consolidates Z.AI descriptor assertions and adds runtime-limit assertions. It does not change product behavior,… |
Full details: Description check
-
Explanation
The description follows the required template. It explains the change and rationale, documents impact, lists testing commands and results, identifies skipped checks, and records the tested provider/model path. No critical section is missing.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
Full details: Risk Surface Disclosed
-
Explanation
PASS. The PR changes only
src/integrations/runtimeMetadata.test.ts; no production auth, routing, permissions, network, startup, plugin, CI, or release code changes. The PR description identifies the Z.AI provider/model path as test-only and states that user-facing impact is none, so no blocker is introduced.
Full details: No Hidden Policy Change
-
Explanation
PASS. The pull request changes only
src/integrations/runtimeMetadata.test.ts. The diff consolidates Z.AI descriptor assertions and adds runtime-limit assertions. It does not change product behavior, trust-model logic, routing defaults, telemetry or network behavior, or permission policy. No maintainer alignment is required for a hidden policy change.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
— jatmn's PR Review agent
Reviewed f696f1028ea80fc271b7657dbf2a2335a9aab7ad against merge-base 78445348676d6c5797fc11d89eaaf0157d69de1b (live-base-three-dot). The diff is a single file: src/integrations/runtimeMetadata.test.ts (+89/−84). No scope creep.
Merge readiness
CI at head (f696f1028ea80fc271b7657dbf2a2335a9aab7ad): All required GitHub checks completed successfully, including typecheck, smoke-and-tests, all eight unit-tests shards, dedicated-tests, launcher compatibility, and pr-intent-scan. Commit status: CodeRabbit — Review completed (success). CodeQL aggregate: No new alerts in code changed by this pull request; CodeQL (javascript-typescript) and CodeQL (actions) succeeded. Open code-scanning alerts on the PR ref: 0.
Branch freshness: Head is 2 commits behind current main (19bb39c8845a6920bb04ad8d70652ad520861b87). Mergeable with no conflict signal; rebase before merge is optional hygiene for the upgrade-plan stack, not a blocker for this test-only change.
Automated review audit (CodeRabbit / CodeQL): No unaddressed actionable bot findings. CodeRabbit ASSERTIVE run on this head reported no actionable inline comments (LGTM on the golden table). No open PR review threads (PRRT). The embedded cr-comment:v1:9e839c57958d0740d47a9b4f note is praise-only — not a restore request (drift: N/A, no PRRT id).
Golden table vs models/*.ts / vendor sources
On the pinned checkout, every golden row matches production literals exactly:
glm-5.3-flashdescriptor — matchessrc/integrations/models/glm.ts(label,classificationincludingvision,capabilities.supportsVision: true,contextWindow1_000_000,maxOutputTokens131_072,runtimeMetadataScope: 'catalog',vendorId/brandId/defaultModel).glm-5.3descriptor — matchesglmModel('glm-5.3', …)in the same file (classificationwithoutvision,supportsVision: false, same windows/scope).- Catalog rows —
catalogLabelvaluesGLM-5.3-Flash/GLM-5.3matchsrc/integrations/vendors/zai.tslabelfields;apiName,modelDescriptorId, reasoning levels (low/high/xhigh,zai_compatible), andtransportOverrides.openaiShim.enableToolStreamingmatch. - Route metadata —
defaultModel: 'glm-5.2'andcatalogSource: 'static'matchzai.ts.
Five loosened assertions (judgment)
| # | Old assertion | New assertion | Verdict |
|---|---|---|---|
| 1 | glmBrand.modelIds[0] === 'glm-5.3-flash' |
toContain('glm-5.3-flash') |
Brittle → OK to drop. No runtime code indexes brand.modelIds; registry cares about membership and duplicate model ids, not brand list order. |
| 2 | glmBrand.modelIds[1] === 'glm-5.3' |
toContain('glm-5.3') |
Brittle → OK to drop. Same as (1). brands/glm.ts still lists both ids in the intended order on head; only the index pin moved. |
| 3 | catalog.models[0] full Flash object |
find(id === 'glm-5.3-flash') + same field asserts |
Brittle → OK to drop index pin. Runtime resolution uses id lookup (resolveOpenAIShimRuntimeContext, catalog find). Field-level contract unchanged. |
| 4 | catalog.models[1].id === 'glm-5.3' |
find-by-id + full entry toMatchObject |
Brittle → OK to drop index pin. Same as (3). |
| 5 | flashEntries.length === 1 (exactly one Flash catalog row) |
presence via find only |
Brittle → OK to drop. validateIntegrationRegistry() in registry.ts emits Duplicate catalog entry id for duplicate ids within a route; index.test.ts runs full registry validation; registry.test.ts has an explicit duplicate-catalog regression. PR claim verified — duplicates are still caught without this local count. |
Not required to restore any of the five for merge. The only user-visible surface that still depends on catalog order is the static model picker (model.tsx iterates catalog.models), but there is no stated product contract that GLM 5.3 variants must occupy indices 0–1 in the zai list (default remains glm-5.2 at a later index). Kimi routes retain stricter order tests elsewhere in runtimeMetadata.test.ts; zai does not.
resolveModelRuntimeLimits coverage
Still holds and is slightly stronger in the golden loop. The consolidated test calls resolveModelRuntimeLimits for both models with zaiVendor.defaultBaseUrl and asserts contextWindow / maxOutputTokens against golden literals. The file already had dedicated it.each blocks for Flash and GLM-5.3 limits (query-string variants, endpoint leakage guards, shim context). No regression — additive wiring in the golden table is consistent with those tests.
Leaving the other seven 0-E-4 files unchanged
Sound. Those suites (registry, routeMetadata, runtimeMetadata.modelLimits, discoveryService, discoveryCache, compatibility, index) embed literals tied to routing, precedence, parsing, cache keys, and cross-route behavior. This PR’s stated slice is D6 trim for the Z.AI GLM-5.3 descriptor/catalog contract only; mass-editing golden tables there would expand scope beyond the upgrade-plan task without improving coverage of the changed hunk.
Findings
🟢 Thanks for the contribution. I do not see any actionable issues from my review.
Summary
src/integrations/runtimeMetadata.test.ts, merge the two Z.AI GLM-5.3 descriptor tests into one golden table keyed by route id (zai) that iterates both models (glm-5.3-flash,glm-5.3).contextWindowandmaxOutputTokensfromsrc/integrations/models/glm.ts. It now also checksresolveModelRuntimeLimitsfor both models against literal golden values, which the old tests didn't.registry,routeMetadata,runtimeMetadata.modelLimits,discoveryService,discoveryCache,compatibility,index) are unchanged. Their catalog literals feed routing, auth, parsing, precedence, cache or registry-validation tests, so trimming them would remove behavior coverage. Registry validation is untouched.Impact
Loosened assertions (all from the old Flash test)
glmBrand.modelIds[0] === 'glm-5.3-flash'is now a membership check, so the index no longer matters.glmBrand.modelIds[1] === 'glm-5.3'is now a membership check.catalog.models[0]matching Flash is now a find-by-id check with the same fields.catalog.models[1].id === 'glm-5.3'is now find-by-id with the full entry assertions.registry is valid after loading all descriptors(index.test.ts), which uses the registry's duplicate-id validation.Testing
bun install --frozen-lockfile: okbun run check: passbun run typecheck: pass.bun run typecheck:type-tests: passnode bin/openclaude --versionandNODE_DISABLE_COMPILE_CACHE=1 node bin/openclaude --version:0.33.0 (OpenClaude)bun run security:pr-scan -- --base FETCH_HEAD --head HEAD: no suspicious additionsgit diff --check: cleanbun test --isolate src/integrations/{registry,routeMetadata,runtimeMetadata,runtimeMetadata.modelLimits,discoveryService,discoveryCache,compatibility,index}.test.ts: 280 pass, 0 fail.bun run integrations:check: artifacts up to date.Notes
zai), GLM-5.3 and GLM-5.3-Flash descriptors (test-only).Summary by CodeRabbit
glm-5.3-flashandglm-5.3models, verifying their catalog details, reasoning and transport settings, and runtime limits of 1,000,000 context tokens and 131,072 output tokens with the Z.AI Coding Plan endpoint.glm-5.2.