Skip to content

feat: add GLM-5.3 Coding Plan support - #103

Merged
byapparov merged 6 commits into
mainfrom
feat/glm-5-3-support
Aug 20, 2026
Merged

feat: add GLM-5.3 Coding Plan support#103
byapparov merged 6 commits into
mainfrom
feat/glm-5-3-support

Conversation

@byapparov

@byapparov byapparov commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Closes #102

Problem

Z.AI released GLM 5.3 and models.dev lists it for Coding Plan, but bundled or stale CLI catalogues stop at GLM 5.2 and do not expose its native reasoning efforts.

The full CI run also exposed a 36-bit timestamp rollover in tool-output cleanup: recent files could be mistaken for expired files when the encoded timestamp wrapped.

Behavior

  • Adds zai-coding-plan/glm-5.3 with a 1M-token context window and 131,072-token output limit.
  • Exposes native low, high, and max efforts plus medium and xhigh compatibility aliases.
  • Updates the model fixture, request coverage, changelog, and patch version to 0.4.2.
  • Reconstructs tool-output timestamps relative to cleanup time so the seven-day retention window remains correct across rollover.
  • Preserves files created during cleanup or after modest clock corrections with a one-hour forward-skew tolerance.
  • Pins the repository AI-review workflow to the published CLI's supported zai-coding-plan/glm-5.2 model.

Scope

Model availability remains limited to Z.AI Coding Plan; this does not advertise GLM 5.3 through gateways that have not published availability or port unrelated OpenCode Go changes. The cleanup changes are limited to timestamp decoding and retention checks. The workflow change only corrects its invalid model selection.

Rollout and risks

No migration or configuration change is required. The model fallback uses ??=, so live catalogue metadata remains authoritative. Request shaping is limited to the new model. Timestamp reconstruction is anchored to cleanup time and is unambiguous within the seven-day retention period; timestamps up to one hour ahead are treated as recent.

Verification

  • bun test test/tool/truncation.test.ts test/provider/transform.test.ts test/session/llm.test.ts — 132 passed.
  • bun run typecheck — passed.
  • actionlint .github/workflows/code-review.yml — passed.
  • pre-push workspace typecheck — 6 tasks passed.
  • git diff --check — passed.
  • bun test — 1,362 passed, 7 skipped, 0 failed.
  • GitHub Actions verify, Aictrl AI Review, and all CodeQL checks — passed.
  • GLM-5.2 workflow re-review — LGTM on head 36ad5f6fa.

@byapparov byapparov self-assigned this Aug 20, 2026
Comment thread packages/cli/src/provider/provider.ts Outdated
Comment thread packages/cli/src/provider/transform.ts
@aictrl-dev

aictrl-dev Bot commented Aug 20, 2026

Copy link
Copy Markdown

Code review

Verdict: Looks good — only minor / nit comments below. · 🔴 0 · 🟠 0 · 🟡 2 · ⚪ 0 · 0/2 resolved

  • 🟡 packages/cli/src/provider/provider.ts:630-634 — Seeded glm-5.3 variants map is dead code in provider.ts
  • 🟡 packages/cli/src/provider/transform.ts:346-352 — glm-5.3 drops medium/xhigh variant aliases glm-5.2 keeps
🤖 Fix all 2 open findings with your agent
Fix the following code review findings on aictrl-dev/cli PR #103 (head branch).
Run the relevant tests/linters after each change.

1. packages/cli/src/provider/provider.ts:630-634 — Seeded glm-5.3 variants map is dead code in provider.ts
   Detail: The same low/high/max reasoningEffort map is declared twice: on the seeded model here (provider.ts:630-634) and as a hardcoded early return in ProviderTransform.variants (transform.ts:346-352). Provider.state()'s init loop unconditionally recomputes model.variants from ProviderTransform.variants(model) for every model, so the seeded map is always discarded and can silently drift from the effective mapping (the same latent duplication already exists for glm-5.2). The effective source of truth is only the transform.ts branch — anyone editing the provider.ts variants alone gets no runtime effect.
   Suggested fix: Pick one authoritative location: either drop the `variants` field from the seeded glm-5.3 entry (or set `variants: {}` like other seeds), keeping ProviderTransform.variants as the single source of truth; or make variants() fall back to `model.variants` when present and delete the hardcoded per-model branch.
2. packages/cli/src/provider/transform.ts:346-352 — glm-5.3 drops medium/xhigh variant aliases glm-5.2 keeps
   Detail: The glm-5.2 branch deliberately aliases medium→high and xhigh→max so persisted/legacy variant keys keep resolving, but the new glm-5.3 branch returns only low/high/max. LLM.stream resolves effort via model.variants[user.variant] (llm.ts:79-80), so a session or agent configured with variant "medium" or "xhigh" that is switched to glm-5.3 resolves undefined — the requested reasoning effort is silently dropped (no reasoningEffort reaches the options merge) instead of being remapped to the nearest supported level. Repro: given a session persisted with user.variant="medium" on glm-5.2, when the model is switched to glm-5.3 and a message is sent, then model.variants["medium"] is undefined and body.thinking carries no remapped effort.
   Suggested fix: Mirror the 5.2 aliasing for backward compatibility: add `medium: { reasoningEffort: "high" }` and `xhigh: { reasoningEffort: "max" }` to the glm-5.3 map, or normalize unknown variant keys to the nearest supported effort in LLM.stream's variant resolution.
📋 Out-of-diff findings (2)
Sev Location Finding
🟡 packages/cli/src/provider/provider.ts:630-634 Seeded glm-5.3 variants map is dead code in provider.ts
🟡 packages/cli/src/provider/transform.ts:346-352 glm-5.3 drops medium/xhigh variant aliases glm-5.2 keeps

Reviewed 7 files · 0 inline · view all 2 findings ↗


aictrl · AI code review for fast-moving teams · aictrl.dev

@byapparov

Copy link
Copy Markdown
Contributor Author

Review response — PR #103

Verified and fixed both GLM-5.3 consistency findings on the existing PR branch.

Issues addressed (pushed to this PR)

  • Seeded GLM-5.3 variants map was dead codepackages/cli/src/provider/provider.ts: removed the duplicate seed map so ProviderTransform.variants() is the single source of truth (commit 293f97141).
  • GLM-5.3 dropped compatibility aliasespackages/cli/src/provider/transform.ts: added medium → high and xhigh → max, with the transform regression covering all five keys (commit 293f97141).

Review claims verified false (no change needed)

None.

Not addressed here

None.

Comment thread packages/cli/src/tool/truncation.ts Outdated
@aictrl-dev

aictrl-dev Bot commented Aug 20, 2026

Copy link
Copy Markdown

Code review

Verdict: Address the major findings before merging. · 🔴 0 · 🟠 1 · 🟡 0 · ⚪ 0 · 0/1 resolved

  • 🟠 packages/cli/src/tool/truncation.ts:40-41 — Cleanup deletes future-dated tool files (clock skew/race)
🤖 Fix all 1 open findings with your agent
Fix the following code review findings on aictrl-dev/cli PR #103 (head branch).
Run the relevant tests/linters after each change.

1. packages/cli/src/tool/truncation.ts:40-41 — Cleanup deletes future-dated tool files (clock skew/race)
   Detail: Identifier.timestamp(id, now) reconstructs "the most recent timestamp at or before reference" (id.ts:86): any entry whose encoded timestamp is even 1ms AFTER `now` is mapped a full TIMESTAMP_CYCLE (~2.17 years) into the past, so `Identifier.timestamp(entry, now) >= cutoff` is false and the file is unlinked immediately. The old wrapped comparison (`Identifier.timestamp(entry) >= Identifier.timestamp(create(...))`) kept future-dated entries, so this is a regression. Trigger 1 (wall-clock step-back, e.g. NTP correction after sleep/VM resume): cleanup runs hourly via Scheduler (truncation.ts:27-34); if Date.now() steps back by N minutes, every tool-output file written in the last N minutes is reconstructed as ~2.17 years old and deleted on the next run. Trigger 2 (race): `now` is captured at cleanup() entry before the awaited Glob.scan, so a tool_<id> file written concurrently by Truncate.output in that window (its ID timestamp > now) is scanned and deleted instantly, breaking the in-conversation truncation hint that tells the agent to Read that file.
   Suggested fix: Treat near-future reconstructions as recent instead of one-cycle-old: in cleanup(), compute the raw cycle value of the entry (e.g. expose Identifier's unwrapped value or compare against `now` directly) and skip deletion when the entry's encoded timestamp exceeds `now` by less than a small clock-skew tolerance (e.g. 1 hour). Alternatively, in cleanup use `reference = now + SKEW_MS` when reconstructing so entries created slightly after `now` still resolve to (nearly) `now` rather than a full TIMESTAMP_CYCLE in the past.
📋 Out-of-diff findings (1)
Sev Location Finding
🟠 packages/cli/src/tool/truncation.ts:40-41 Cleanup deletes future-dated tool files (clock skew/race)

Reviewed 11 files · 0 inline · view all 1 findings ↗


aictrl · AI code review for fast-moving teams · aictrl.dev

@github-actions

Copy link
Copy Markdown

Review

Verified the timestamp rollover fix and the GLM-5.3 addition, including running the affected suites locally (truncation, transform, session/llm — 131 tests, 0 failures).

Timestamp rollover fix — correct

  • The encoding stores (ts * 2^12 + counter) mod 2^48, so timestamp() recovers ts mod 2^36 (cycle ≈ 2.18 years). Reconstructing relative to a reference is unambiguous for the 7-day retention window, and I confirmed the math for the same-cycle, cross-boundary, and expired cases, including the test's boundary scenario (10-day-old file lands in the previous cycle).
  • Identifier.timestamp has exactly one caller (Truncate.cleanup), so the semantic change is fully contained.
  • Replacing the synthesized-ID cutoff with plain now - RETENTION_MS arithmetic also removes the old cutoff's dependency on the same wrapped decoding — good.

GLM-5.3 addition — correct, consistent with 5.2

  • Model entry mirrors glm-5.2 (URL, @ai-sdk/openai-compatible, limits, reasoning_content interleaved field); ??= keeps a live catalogue authoritative.
  • The variants() branch is placed before the generic glm{} catch-all, so ordering is right. medium → high matches the 5.2 compat-alias pattern.
  • Note: the glm-5.3 entry omits the inline variants field that glm-5.2 carries in provider.ts. This is harmless — provider.ts:1110 unconditionally regenerates variants from ProviderTransform.variants(model) — but it means the inline variants on the glm-5.2 entry (~line 678) is dead code. Worth dropping in a follow-up for consistency.

Non-blocking observation

  • timestamp() maps an encoded value slightly ahead of reference back a full cycle (~2.18 years). An entry minted with a clock a few ms/minutes ahead of the cleanup clock (NTP step-back, or a shared data dir across machines with skew) would be reconstructed as ~2.18 years old and deleted on the first cleanup pass. Practical risk is low given hourly single-machine cleanup, but if that ever matters, clamping small positive deltas instead of subtracting a full cycle would harden it.

Workflow

  • Bumping the review workflow to glm-5.2 (rather than the newly added glm-5.3) is reasonable — 5.3 is brand new and 5.2 is proven.

No security concerns: cleanup unlinks are scoped to tool_* inside the data dir, no new inputs flow into paths or shells.

Reviewed SHA: 6a50d38

@byapparov

Copy link
Copy Markdown
Contributor Author

Review response — PR #103

Verified and fixed the future-timestamp cleanup regression on the existing PR branch.

Issues addressed (pushed to this PR)

  • Cleanup deleted future-dated tool filespackages/cli/src/tool/truncation.ts: timestamp reconstruction now allows one hour of forward clock skew, and a regression proves files created just after cleanup starts are preserved (commit 36ad5f6fa).

Review claims verified false (no change needed)

None.

Not addressed here

None.

@github-actions

Copy link
Copy Markdown

Review

Verdict: LGTM. The 36-bit rollover fix is real, correct, and timely; the GLM-5.3 additions are consistent with the existing GLM-5.2 template. Verified by running bun test test/tool/truncation.test.ts test/provider/transform.test.ts test/session/llm.test.ts (132 pass) and by checking the reconstruction math at the actual cycle boundary.

Timestamp rollover fix (id.ts / truncation.ts)

  • Confirmed the 2^36 ms cycle boundary lands on 2026-08-14 — i.e., the rollover already happened six days ago. Under the old code, any cleanup run after Aug 14 compares wrapped values: files created after the boundary wrap to a small value while the 7-day cutoff (computed from before the boundary) wraps to a value near 2^36, so recent tool-output files get deleted. This PR is a genuine bug fix, not theoretical.
  • The modular reconstruction (base + value, minus one cycle if > reference) is the standard approach and is exact for any ID up to one cycle (~2.18 years) old — far beyond the 7-day retention window, so even stale entries past one cycle still land on the "delete" side of the cutoff.
  • The now + HOUR_MS reference guard correctly handles the race where a file is written concurrently with cleanup (embedded ts slightly after now): without it, such a file would reconstruct as ~2.18 years old and be deleted immediately. Good catch, and the new regression test covers it.
  • The semantic change to Identifier.timestamp() is safe: truncation.ts is its only caller, and removing the old Identifier.create(...) call from cleanup also stops perturbing the module's monotonic counter state — cleaner.
  • The wrap-boundary test uses a deterministic cycle-derived now instead of Date.now(), so it will keep exercising the wrap regardless of when it runs. Nice.

GLM-5.3 additions (provider.ts / transform.ts / fixtures)

  • Model entry mirrors glm-5.2 (same API URL, npm package, 1M/131072 limits) and uses ??=, so a live models.dev catalogue remains authoritative. Correct.
  • The variants branch in transform.ts is placed before the generic id.includes("glm") → {} early return — necessary and correct. medium/high both mapping to high mirrors the existing glm-5.2 pattern.
  • The parameterized llm.test + new models-api.json fixture properly cover the wire payload (body.model, thinking, reasoningEffort: "max") for both models.
  • The glm-5.3 fallback omits the inline variants key that glm-5.2's entry carries — harmless, since variants are recomputed via ProviderTransform.variants() in all provider-load paths (provider.ts:843/985/1110). The inline copy on glm-5.2 is arguably the redundant one.

Minor notes (non-blocking)

  1. Clock-skew window: a file whose embedded timestamp is > 1h ahead of cleanup now (e.g., system clock stepped back > 1h between write and cleanup) still reconstructs as ancient and is deleted. The 1h guard matches the scheduler interval, so this seems like a deliberate, reasonable tradeoff — just flagging the residual edge.
  2. Pre-existing robustness gap (not a regression): a non-conforming file matching tool_* in the tool-output dir (e.g., tool-notes.txt) makes BigInt("0x" + hex) throw inside the cleanup loop, aborting the pass for remaining entries. Optional hardening: skip entries whose ID doesn't parse.
  3. Semver: the CHANGELOG entry is under "Features" but ships as a patch bump (0.4.1 → 0.4.2). Process nit only.

Reviewed SHA: 36ad5f6

@byapparov
byapparov merged commit 2dc4702 into main Aug 20, 2026
5 checks passed
@byapparov
byapparov deleted the feat/glm-5-3-support branch August 20, 2026 10:39
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.

Add GLM 5.3 to the Z.AI Coding Plan catalogue

1 participant