From 2973df47bfa0f68f152a0bffd03a2ac68c72f53b Mon Sep 17 00:00:00 2001
From: JUN <bitkyc08@gmail.com>
Date: Thu, 17 Sep 2026 17:45:55 +0900
Subject: [PATCH] docs(devlog): plan the L6 contract and quality fix lane

Six narrow contract defects that share no code path, one pull request each.
Records what each unit may not change, and two repository gates checked against
the tree rather than assumed: the file-size ratchet pins gui/src/pages/Models.tsx
at its current 2792 lines, so the carried custom-model validation cannot add a
net line, and enforce-target requires a GUI screenshot this lane cannot produce.
---
 .../000_master_plan.md                        |  78 ++++++++++++++
 .../010_4855_terminal_stop_classification.md  |  92 ++++++++++++++++
 .../020_4822_zai_model_discovery.md           |  71 +++++++++++++
 .../030_carried_catalog_and_gui_prs.md        | 100 ++++++++++++++++++
 .../040_4857_first_frame_usage_contract.md    |  83 +++++++++++++++
 5 files changed, 424 insertions(+)
 create mode 100644 devlog/_plan/260917_l6_contract_quality_fixes/000_master_plan.md
 create mode 100644 devlog/_plan/260917_l6_contract_quality_fixes/010_4855_terminal_stop_classification.md
 create mode 100644 devlog/_plan/260917_l6_contract_quality_fixes/020_4822_zai_model_discovery.md
 create mode 100644 devlog/_plan/260917_l6_contract_quality_fixes/030_carried_catalog_and_gui_prs.md
 create mode 100644 devlog/_plan/260917_l6_contract_quality_fixes/040_4857_first_frame_usage_contract.md

diff --git a/devlog/_plan/260917_l6_contract_quality_fixes/000_master_plan.md b/devlog/_plan/260917_l6_contract_quality_fixes/000_master_plan.md
new file mode 100644
index 0000000000..de97768d19
--- /dev/null
+++ b/devlog/_plan/260917_l6_contract_quality_fixes/000_master_plan.md
@@ -0,0 +1,78 @@
+# L6 — small contract and quality fixes
+
+Base: `origin/dev` at `f1dfda8e48` (#4876 merged). Package 2.58.0.
+
+This lane collects six narrow contract defects that share no code path. Each one
+ships as its own pull request against `dev`. Nothing here is a refactor, a
+generalization, or a CI change — the CI stabilization round closed before this
+lane opened.
+
+## Units
+
+| Unit | Source | Surface | Doc |
+| --- | --- | --- | --- |
+| U1 | issue #4855 | `src/bridge/sse.ts`, `src/bridge/response-json.ts` | [010](./010_4855_terminal_stop_classification.md) |
+| U2 | issue #4822 | `src/providers/model-discovery.ts`, `zai` registry row | [020](./020_4822_zai_model_discovery.md) |
+| U3 | PR #4788 | Alibaba Token Plan catalogs | [030](./030_carried_catalog_and_gui_prs.md) |
+| U4 | PR #4802 | `src/server/models-capabilities.ts` | [030](./030_carried_catalog_and_gui_prs.md) |
+| U5 | PR #4863 | `gui/src/pages/Models.tsx` | [030](./030_carried_catalog_and_gui_prs.md) |
+| U6 | issue #4857 | `src/claude/outbound.ts` | [040](./040_4857_first_frame_usage_contract.md) |
+
+## Ordering
+
+U1 goes first and waits on nothing. It is the only unit a user sees on every
+Anthropic-routed turn, the fix is two predicates, and both call sites already
+import the classifier it needs.
+
+U6 goes last. It touches the same subject area as U1 — what a terminal frame is
+allowed to claim — but a different file and a different contract, so treating
+them as one unit would only make the visible defect wait for the harder one.
+
+U2 through U5 are mutually independent and can land in any order. The only
+coupling worth recording is textual: U2 edits the `zai` row and U3 edits the
+two `alibaba-token-plan` rows, both in
+`src/providers/registry/entries-extended.ts` and
+`src/providers/registry/model-seeds.ts`. The rows are hundreds of lines apart
+and neither reads the other's constants, so this is a merge-order note for the
+host, not a dependency.
+
+## Constraints this lane operates under
+
+No local verification of any kind. No `bun test`, `bun run test:changed`,
+`bun run typecheck`, `bun install`, `bun run build:gui`, or `ocx`
+invocation. Every claim in these documents comes from reading the source at the
+stated commit; every claim about whether a change is correct comes from hosted
+CI at an exact head.
+
+Pushes use `--no-verify` because the pre-push hook runs the local suite.
+
+This lane never merges, never pushes to `dev`, and never rebases without being
+told to. A unit is done when its PR is open and hosted CI has reported at its
+exact head.
+
+## Repository gates that bind these units
+
+Two gates are load-bearing here and were checked against the tree rather than
+assumed.
+
+`scripts/file-size-ratchet.ts` caps every tracked file at its recorded line
+count once the file is at or over 2000 lines. `gui/src/pages/Models.tsx` is in
+`tests/fixtures/file-size-baseline.json` at 2792 and currently measures 2792,
+so U5 cannot add a net line to it. The carried diff is +10/-4. This is resolved
+in [030](./030_carried_catalog_and_gui_prs.md), not deferred.
+
+`enforce-target` requires a screenshot in the description of any PR whose title
+or body mentions `gui`. U5 is a GUI change and this lane cannot build the GUI,
+so it cannot produce one. That is reported to the host rather than worked
+around.
+
+## Attribution
+
+U3, U4 and U5 carry work authored by @oliver-mee, @Yum-wu and @codingbooo
+respectively. Each carried branch gets a `Co-authored-by` trailer in a branch
+commit so it survives the squash, per the "Landing another author's work" rule
+in `AGENTS.md`. Prose credit is not a substitute and is not used here.
+
+The carry runs on `codex/` branches rather than by pushing into the
+contributors' forks. The original pull requests stay open and untouched; the
+host decides which of the two lands.
diff --git a/devlog/_plan/260917_l6_contract_quality_fixes/010_4855_terminal_stop_classification.md b/devlog/_plan/260917_l6_contract_quality_fixes/010_4855_terminal_stop_classification.md
new file mode 100644
index 0000000000..7234ccd956
--- /dev/null
+++ b/devlog/_plan/260917_l6_contract_quality_fixes/010_4855_terminal_stop_classification.md
@@ -0,0 +1,92 @@
+# U1 — a clean `end_turn` must still produce `final_answer`
+
+Source: issue #4855. Verified against `f1dfda8e48`.
+
+## What is wrong
+
+The Responses bridge decides whether a terminal assistant message is the final
+answer by testing whether `stopReason` is truthy:
+
+```ts
+// src/bridge/sse.ts:1174
+if (currentMsg) closeCurrentMessage(event.stopReason ? undefined : "final_answer");
+```
+
+Anthropic ends a normal turn with `stop_reason: "end_turn"` and the adapter
+forwards that string verbatim, so a successful turn takes the `undefined`
+branch and the terminal message ships without a phase. In Codex App the turn
+then renders without the divider that separates activity from the answer.
+
+The non-streaming path splits the same way:
+
+```ts
+// src/bridge/response-json.ts:530
+cleanDone = e.stopReason === undefined;
+```
+
+`cleanDone` is the only input to the `flushText(...)` phase decision further
+down, so the buffered path drops the phase for exactly the same reason.
+
+## Why the classifier is the right answer
+
+`src/responses/truncated-stop-reason.ts` exists for this. Three of the four
+decisions in the same `case "done":` block already call it — lines 1179, 1184
+and 1196 — and 1174 is the one that does not. On the buffered side, lines 538
+and 558 already call `truncationReasonFor` and `isTruncatedStopReason`, and
+530 is the one that does not.
+
+`TRUNCATED_STOP_REASONS` maps Anthropic's `refusal`, `pause_turn`,
+`max_output_tokens` and `model_context_window_exceeded`. It deliberately does
+not map `end_turn`, `stop_sequence` or `tool_use`, and its header states that
+unknown reasons are not truncation.
+
+## What must not change
+
+A clean stop and a cut-short stop must stay distinguishable. This unit is not
+"treat every terminal as final" — it is "ask the classifier instead of asking
+whether the string is non-empty". Concretely, after the change:
+
+- `end_turn`, `stop_sequence`, `tool_use` and an absent `stopReason` close
+  the message as `final_answer`.
+- Every value in `TRUNCATED_STOP_REASONS` still closes without a phase, still
+  fails an open tool call, still marks an in-flight search `failed`, and still
+  suppresses the compaction item.
+- The `error` and `incomplete` terminals are untouched. On the buffered path
+  the existing `cleanDone && !errorEvent && !incompleteEvent` conjunction
+  already covers them and stays as written.
+
+## Change
+
+Two predicates:
+
+```diff
+- if (currentMsg) closeCurrentMessage(event.stopReason ? undefined : "final_answer");
++ if (currentMsg) closeCurrentMessage(isTruncatedStopReason(event.stopReason) ? undefined : "final_answer");
+```
+
+```diff
+- cleanDone = e.stopReason === undefined;
++ cleanDone = !isTruncatedStopReason(e.stopReason);
+```
+
+`isTruncatedStopReason` is already imported in both files. No new coupling.
+
+## Regression coverage
+
+`tests/adapters/bridge.test.ts` already drives both `bridgeToResponsesSSE` and
+`buildResponseJSON` and is 1683 lines, below the 2000-line ratchet threshold.
+Adding there needs no `scripts/test-layout/layout.json` or
+`tests/fixtures/test-layout-expected.json` entry, so this unit adds no test
+file.
+
+Four cases, both entry points:
+
+1. `done` with `stopReason: "end_turn"` and open text closes the message with
+   `phase: "final_answer"`.
+2. `done` with `stopReason: "max_output_tokens"` closes it with no phase.
+3. `done` with `stopReason: "refusal"` closes it with no phase, and the turn
+   still reports `content_filter`.
+4. `done` with no `stopReason` keeps its existing `final_answer` behaviour.
+
+Case 2 and case 3 are the ones that would catch an over-broad fix, and they are
+the reason this unit is not a one-line patch with no test.
diff --git a/devlog/_plan/260917_l6_contract_quality_fixes/020_4822_zai_model_discovery.md b/devlog/_plan/260917_l6_contract_quality_fixes/020_4822_zai_model_discovery.md
new file mode 100644
index 0000000000..69639d3bdd
--- /dev/null
+++ b/devlog/_plan/260917_l6_contract_quality_fixes/020_4822_zai_model_discovery.md
@@ -0,0 +1,71 @@
+# U2 — Z.AI discovery needs both the endpoint and the envelope
+
+Source: issue #4822. Verified against `f1dfda8e48`.
+
+## The trap in this issue
+
+There are two defects and fixing either one alone leaves discovery broken.
+
+The `zai` registry row carries `baseUrl: "https://api.z.ai"` and no
+`modelDiscovery` spec, so `providerModelsUrl` builds
+`https://api.z.ai/models`, which the reporter observed as an nginx 404. The
+row already knows the real prefix — it sets `responsesPath: "/api/v1/responses"`
+— and `src/providers/registry/model-seeds.ts` already records
+`GET https://api.z.ai/api/v1/models` as the authoritative roster URL in a
+comment. Only the discovery URL disagrees with the rest of the row.
+
+Correcting the URL alone still fails. `extractProviderModelItems` accepts one
+envelope key, `data`, or a top-level array, and reads each row's `id`. Z.AI
+returns `{"models": [{"slug": "glm-5.3"}, ...]}`, so the parser answers
+`{ ok: false, reason: "invalid_shape" }` and the dashboard reports the same
+failure it reported before, with a different cause.
+
+## The opposite trap
+
+This is also not a reason to generalize the discovery contract. The single
+allowlisted `data` envelope is deliberate: the code comment at line 505 records
+that a bare `models` key on an openai-chat response is specifically *not*
+accepted, and `buildSiblingIndex` already uses a `models[]` sibling for a
+different purpose — enriching rows that entered through `data[]`. Teaching the
+shared parser to accept `models[].slug` for everybody would change what a
+`models` key means for every provider that sends one, and llama.cpp's
+dual-envelope body is served by exactly that distinction.
+
+So the envelope and identifier widening is scoped to the provider that needs it,
+through the existing per-provider `modelDiscovery` spec, not by relaxing the
+default.
+
+## Change
+
+Two edits, one provider.
+
+1. `src/providers/registry/entries-extended.ts`: give the `zai` row a
+   `modelDiscovery` spec pinning the path to `/api/v1/models`. The `path`
+   form resolves against the registry's own `baseUrl`, which
+   `isRegistryModelDiscoveryUrl` already treats as canonical, so the
+   URL-allowlist check keeps working. `baseUrl` itself is not touched —
+   `responsesPath`, `chatCompletionsPath` and `destinationAliases` all resolve
+   against it and changing it would move the inference wires.
+2. `src/providers/model-discovery.ts`: let a `modelDiscovery` spec declare the
+   envelope key and the identifier field it expects, and apply that in
+   `extractProviderModelItems` instead of the hard-coded `["data"]` /
+   `id` pair. Providers without a spec keep the current behaviour byte for
+   byte.
+
+The `models` static seed stays. Discovery failing back to a seeded roster is
+the behaviour `liveModels` already relies on, and the seed is what keeps the
+picker populated while the live call is in flight.
+
+## Regression coverage
+
+`tests/providers/provider-model-discovery-contract.test.ts` is the existing home
+for both halves, so no new test file and no layout entry.
+
+1. `resolveProviderModelDiscoveryUrl("zai", ...)` yields
+   `https://api.z.ai/api/v1/models`, and `isRegistryModelDiscoveryUrl` accepts
+   that URL and rejects `https://api.z.ai/models`.
+2. `extractProviderModelItems` on `{"models":[{"slug":"glm-5.3"}]}` with the
+   `zai` spec returns the model, and the same body with no spec still returns
+   `invalid_shape`. The second assertion is what keeps this scoped.
+3. The inference paths are unchanged: the `zai` row still resolves
+   `/api/v1/responses` and the Chat alias after the spec is added.
diff --git a/devlog/_plan/260917_l6_contract_quality_fixes/030_carried_catalog_and_gui_prs.md b/devlog/_plan/260917_l6_contract_quality_fixes/030_carried_catalog_and_gui_prs.md
new file mode 100644
index 0000000000..bc674030b2
--- /dev/null
+++ b/devlog/_plan/260917_l6_contract_quality_fixes/030_carried_catalog_and_gui_prs.md
@@ -0,0 +1,100 @@
+# U3, U4, U5 — three carried contributor pull requests
+
+Verified against `f1dfda8e48` and against each PR head on 2026-09-17.
+
+All three originals have `maintainer_can_modify: true`, so pushing into the
+contributor forks is technically available. This lane does not use it. Writing
+into someone else's repository is a side effect the host did not ask for, and
+`AGENTS.md` already documents the alternative: carry the work onto a `codex/`
+branch with a `Co-authored-by` trailer in a branch commit, where it survives
+the squash. The originals stay open and unmodified.
+
+## U3 — Alibaba Token Plan catalog refresh (PR #4788, @oliver-mee)
+
+Head `fce03915e04e9128ce65c4523358252e4bb6f5fd`, draft, closes #4787. Touches
+`src/providers/registry/model-seeds.ts`,
+`src/providers/registry/entries-extended.ts`, and three test files.
+
+This is a reuse, not a rewrite. The values in it are gateway probes with dates
+and a stated method — accept at N, reject at N+1 for every
+`modelMaxOutputTokens` row — and that evidence cannot be reconstructed from
+here without making live calls. Rebuilding the catalog from scratch would throw
+away the only part of the change that is expensive.
+
+What the carry does: replay the branch onto current `dev`, keep every catalog
+value as the author probed it, and add the trailer. The author's own report
+names the one thing to re-check after the replay —
+`tests/providers/provider-registry-parity.test.ts` pins the Beijing contract and
+is updated in the same commit, so a replay that drops that file leaves the
+suite red.
+
+Two questions the original review raised stay open and stay out of scope: the
+per-tier split where a Personal Edition subscription sees Team rows and gets a
+403, and the display alias for the long plan slugs. Both are registry design,
+not catalog data.
+
+## U4 — model capacity on the `/v1/models` top level (PR #4802, @Yum-wu)
+
+Head `14c478cda201c82b9a5d9f3dd6460c7fcdab4c03`, ready for review, 24 added and
+3 removed lines in `src/server/models-capabilities.ts` plus 33 added lines in
+`tests/providers/cursor/cursor-local-models-schema.test.ts`.
+
+The change mirrors `context_window` and `max_output_tokens` onto the top level
+of each model row, beside the nested `capabilities` object Cursor reads, so a
+client that reads only the top level stops seeing a model with no declared
+capacity. Keys are omitted rather than zeroed when the value does not pass
+`positiveInt`, which is the part that matters: a `0` or `NaN` on the top level
+would be worse than an absent key.
+
+The gap is the long-tier row. When a model has both `contextWindow` and
+`longContextWindow`, the mirrored value follows `effectiveContextLength`, so
+the top level advertises the long window. The submitted tests cover the flat
+input and the empty input and never pin that case, which leaves the actual
+policy decision unrecorded. The carry adds the assertion for the behaviour as
+implemented, so the choice is visible in the suite rather than implied by it,
+and a line to the module header saying the top-level keys exist for external
+clients.
+
+Whether the long window or the base window is the right thing to advertise is a
+product decision for the host. This unit records the current answer; it does not
+change it.
+
+## U5 — custom-model context window validation (PR #4863, @codingbooo)
+
+Head `e2d2017ba5ab50dbee1b787d45081febac50d3f9`, draft, 21 commits behind
+`dev` as of the review. Touches `gui/src/pages/Models.tsx` (+10/-4) and adds
+`gui/tests/models-custom-context-invalid.test.tsx` (+295).
+
+The defect is a silent success. Typing `350k` into the Custom Model dialog —
+the same k-suffixed form the UI itself renders through `fmtK` — produces
+`Number("350k") === NaN`, so the field is dropped on add or sent as `null` on
+edit, and the dialog closes with a success toast. The provider-level context
+dialog in the same file already handles this through
+`parseContextWindowDraft`, which returns `null` for empty, a number for a
+positive safe integer, and `undefined` for anything else. The fix routes the
+custom dialog through the same parser and surfaces `models.contextInvalid` in
+the existing `customError` notice.
+
+### The ratchet blocks the diff as written
+
+`gui/src/pages/Models.tsx` is recorded in
+`tests/fixtures/file-size-baseline.json` at 2792 lines and measures 2792 now.
+`scripts/file-size-ratchet.ts` returns `GREW` for any file above its recorded
+cap, and `tests/ci-workflows/file-size-ratchet.test.ts` fails on it. A net `+6`
+is a CI failure, not a warning.
+
+The carry therefore lands the same behaviour without growing the file. The
+validation is one early return and a reuse of an existing parser and an existing
+error string; expressing it within the lines the current block already occupies
+is a formatting constraint, not a design compromise. Raising the baseline is not
+an option — the baseline only ever moves down, by
+`Math.min(cap, lines)` in `updateBaseline`.
+
+### The screenshot gate
+
+`enforce-target` requires a screenshot in the description of any PR whose title
+or description mentions `gui`, and the original PR is already held in draft by
+exactly this. Producing one means building and running the GUI, which this lane
+is forbidden to do. The unit therefore stops with the branch pushed and the PR
+open, and the missing screenshot is reported to the host as a blocker the host
+has to clear.
diff --git a/devlog/_plan/260917_l6_contract_quality_fixes/040_4857_first_frame_usage_contract.md b/devlog/_plan/260917_l6_contract_quality_fixes/040_4857_first_frame_usage_contract.md
new file mode 100644
index 0000000000..92714bf725
--- /dev/null
+++ b/devlog/_plan/260917_l6_contract_quality_fixes/040_4857_first_frame_usage_contract.md
@@ -0,0 +1,83 @@
+# U6 — what `message_start` is allowed to claim about usage
+
+Source: issue #4857. Verified against `f1dfda8e48`.
+
+## What is wrong, stated precisely
+
+`messageSnapshot(model)` in `src/claude/outbound.ts` hard-codes
+`usage: { input_tokens: 0, output_tokens: 0 }`, and `ensureStarted()` emits
+`message_start` with that snapshot. Real usage reaches the client only through
+`anthropicUsage(...)` on the terminal `message_delta`.
+
+This is not an accounting defect. `~/.opencodex/usage.jsonl` records the right
+numbers, Claude Code does not read the first frame, and compaction and cost are
+unaffected. It is a display-contract defect: real Anthropic populates
+`message_start.message.usage.input_tokens` with the prompt size, a third-party
+client that follows that documented contract reads `0`, and Paseo's context
+ring shows a few hundred tokens for a 97k-token session.
+
+## Two things this unit must not do
+
+It must not manufacture a number that is not known when `message_start` is
+emitted. An estimate is indistinguishable from a measurement once it is on the
+wire, and a client that trusts the contract would then be wrong in a new way
+instead of the old one.
+
+It must not buffer the response to learn the usage before emitting the first
+frame. That trades a display gap for a latency regression on every turn, and
+the streaming surface exists precisely so the client sees output early.
+
+Both are ruled out, so "always correct `input_tokens` in `message_start`" is
+not an achievable goal and is not the completion criterion.
+
+## The seam that makes a real fix possible anyway
+
+`message_start` is already lazy. `ensureStarted()` is not called when the
+stream opens — it is called from the first event that produces output, and from
+`finish()`. So any usage the upstream has already reported by the time the
+first content arrives is in hand before the frame is written. Using it is not
+buffering and not estimation; it is reading a value that arrived first.
+
+That splits the upstreams into two populations, and the policy differs by
+population rather than by guesswork:
+
+**Early-confirmed usage.** The upstream reported input usage before the first
+content event. `message_start` carries the real `anthropicUsage(...)` values,
+including `cache_read_input_tokens` and `cache_creation_input_tokens`. A
+first-frame reader is correct from the first frame.
+
+**No early usage.** The upstream has reported nothing by then, which the issue
+correctly identifies as the common case for the Responses path.
+`message_start` keeps the zeroed snapshot, because the Anthropic wire shape
+requires the key and there is no honest value to put in it. Nothing is invented
+and nothing is delayed. The terminal `message_delta` stays authoritative, as it
+is today.
+
+The two populations must not contradict each other or the final accounting.
+Concretely: whatever `message_start` claims, the terminal `message_delta`
+still carries the full `anthropicUsage(...)` result for the turn, and
+`usage.jsonl` is unchanged by this unit. A client that reads only the last
+frame sees exactly what it sees today.
+
+## Regression coverage
+
+`tests/claude-integration/claude-outbound.test.ts` is the existing home, so no
+new test file and no layout entry.
+
+The current suite asserts usage almost entirely on `message_delta`, which is
+why a zeroed first frame never registered as a regression. The new assertions
+pin the split rather than a constant:
+
+1. Upstream reports input usage before the first content event: `message_start`
+   carries that input count, and cache read/creation values survive the
+   `anthropicUsage` transform.
+2. Upstream reports usage only at the end: `message_start` carries the zeroed
+   snapshot and the terminal `message_delta` carries the real numbers. This
+   case is asserted as the documented policy for an unknowable value, not as
+   the correct output of the translator in general.
+3. In both cases the terminal `message_delta` reports the same totals it
+   reports today, so display and accounting cannot disagree.
+
+Case 2 is deliberately worded in the test so that a later change which starts
+estimating the first frame has to delete an assertion that says why it was
+zero, rather than silently flipping a number.