P3 internal#718: canvas consumes registry-served /templates, retire hardcoded provider vocab #4/#5 (PR-B; NOT merged) #1978

Merged

hongming merged 2 commits from feat/internal-718-p3b-canvas-consume-registry into main 2026-05-28 05:59:08 +00:00

Conversation 1 Commits 2 Files Changed 4

+404 -14

hongming commented 2026-05-28 02:16:26 +00:00

Owner

Copy Link

Copy Source

internal#718 P3 — PR-B (canvas, no-merge): consume the registry-served /templates list, retire hardcoded provider vocab (#4/#5). Base = PR-A branch (feat/internal-718-p3a-templates-from-registry); re-target to main after PR-A (#1977) merges.

Second of three stacked P3 PRs. UI-affecting → CTO merge-go after the Five-Axis gate. NOT merged.

What this does (retire-list #4 / #5)

The canvas Provider/Model selector + Config-tab billing-mode now consume the registry-served GET /templates fields (registry_backed / registry_providers / registry_models from PR-A) instead of re-deriving provider knowledge client-side:

• ProviderModelSelector (#4): new buildProviderCatalogFromRegistry(providers, models) builds the dropdown catalog from the registry payload — provider label = registry display_name, bucket = DERIVED provider, billing_mode + auth_env from the registry — instead of inferVendor / VENDOR_LABELS / BARE_VENDOR_PATTERNS. The selector takes an optional pre-built catalog prop and uses it verbatim when supplied. The heuristic (inferVendor/buildProviderCatalog) remains ONLY as the fallback for non-registry runtimes / older backends.
• ConfigTab (#5): when the selected runtime is registry-backed, the provider catalog + selector models come from registry_providers/registry_models, and billingModeForSelectedProvider(provider, catalog) reads the DERIVED provider's billing_mode off the registry catalog. The hardcoded billingModeForProvider ('' | 'platform' → platform_managed else byok) stays as the fallback only. So the billing-mode the UI shows/sends reflects the DERIVED provider (folds in the closed #1931's canvas intent).

Constraints

• Federation/back-compat preserved: a non-registry runtime (external/mock/kimi/future third-party) or an older backend yields registry_backed=false → the canvas keeps the template-served models + its heuristic, unchanged.
• NO hard-reject — the canvas just can't render an option the registry didn't serve for registry-backed runtimes.
• Out of scope (per brief): the manifest runtime allowlist (SUPPORTED_RUNTIME_VALUES / FALLBACK_RUNTIME_OPTIONS) is NOT a provider vocabulary and is untouched. PUT /workspaces/:id/provider is NOT retired (that CTO-decision-#3 follow-through is a later phase).

Tests / build (Phase 3)

• TDD: ProviderModelSelector.registry.test.tsx (catalog bucketed by derived provider, labelled from display_name, carries billing_mode + auth_env, no empty buckets); ConfigTab.registryBilling.test.tsx (billing reads registry catalog; falls back to the legacy rule with no catalog / unknown provider).
• Full canvas suite green: 223 files, 3380 passed / 1 skipped. tsc --noEmit clean for the touched files (pre-existing unrelated vitest-mock type errors in ContextMenu/EmptyState/OrgCancelButton/SidePanel are untouched). eslint 0 issues on touched files (also removed a pre-existing availableModels exhaustive-deps warning by memoizing it).

Five-Axis (Phase 4)

Walked all five — Correctness (registry path + heuristic fallback both tested; un-annotated models skipped from the cascade), Readability (no finding), Architecture (heuristic kept as clean federation/back-compat fallback), Security (NAMES-only auth_env, no secrets), Performance (catalog memoized; no new fetch — rides the existing /templates call). Approve.

cross-ref: internal#718 P3. Stacked on PR-A #1977; PR-C = controlplane provisioner (#2).

---

SOP checklist (RFC#351 — peer /sop-ack to satisfy each)

• Comprehensive testing performed: 2 new vitest files — ProviderModelSelector.registry.test.tsx (catalog bucketed by DERIVED provider, labelled from registry display_name, carries billing_mode + auth_env, no empty buckets) and ConfigTab.registryBilling.test.tsx (billing reads the registry catalog; falls back to the legacy rule with no catalog / unknown provider). Full canvas suite green: 223 files, 3380 passed / 1 skipped. Edge cases: non-registry runtime / older backend (registry_backed=false → heuristic fallback), stale saved provider not in catalog (legacy-rule fallback).
• Local-postgres E2E run: N/A — pure-frontend change (canvas TSX only; no server/DB code path).
• Staging-smoke verified or pending: scheduled post-merge — after deploy, open a claude-code workspace's Config tab and confirm the Provider dropdown labels + billing-mode come from the registry payload (not VENDOR_LABELS), and a non-registry runtime still renders via the heuristic.
• Root-cause not symptom: the canvas re-derived provider knowledge client-side (inferVendor/VENDOR_LABELS/BARE_VENDOR_PATTERNS + a hardcoded billingModeForProvider rule) instead of consuming the registry SSOT; this consumes the registry-served /templates fields so the provider vocab + billing-mode come from the DERIVED provider.
• Five-Axis review walked: Correctness (registry path + heuristic fallback both tested; un-annotated models skipped from the cascade), Readability (no finding), Architecture (heuristic retained as a clean federation/back-compat fallback, not duplicated logic), Security (NAMES-only auth_env in the catalog; no secret values), Performance (catalog memoised; no new fetch — rides the existing /templates call). Approve.
• No backwards-compat shim / dead code added: No — the inferVendor/buildProviderCatalog heuristic is RETAINED ON PURPOSE as the fallback for non-registry runtimes + older backends (federation/back-compat, not a shim). No dead code; the selector's new optional catalog prop is consumed by ConfigTab.
• Memory/saved-feedback consulted: feedback_canvas_only_reporting_default (UI-surface change), feedback_pr_merge_conventions (not merged; merge-commit on go), reference_providers_runtime_matrix_ssot (canvas now consumes the SSOT-derived list). Removed a pre-existing availableModels exhaustive-deps eslint warning while touching the file.

**internal#718 P3 — PR-B (canvas, no-merge): consume the registry-served /templates list, retire hardcoded provider vocab (#4/#5).** Base = PR-A branch (`feat/internal-718-p3a-templates-from-registry`); **re-target to `main` after PR-A (#1977) merges.** Second of three stacked P3 PRs. UI-affecting → **CTO merge-go after the Five-Axis gate.** **NOT merged.** ### What this does (retire-list #4 / #5) The canvas Provider/Model selector + Config-tab billing-mode now consume the registry-served `GET /templates` fields (`registry_backed` / `registry_providers` / `registry_models` from PR-A) instead of re-deriving provider knowledge client-side: - **ProviderModelSelector (#4):** new `buildProviderCatalogFromRegistry(providers, models)` builds the dropdown catalog from the registry payload — provider label = registry `display_name`, bucket = DERIVED provider, `billing_mode` + `auth_env` from the registry — instead of `inferVendor` / `VENDOR_LABELS` / `BARE_VENDOR_PATTERNS`. The selector takes an optional pre-built `catalog` prop and uses it verbatim when supplied. The heuristic (`inferVendor`/`buildProviderCatalog`) remains ONLY as the fallback for non-registry runtimes / older backends. - **ConfigTab (#5):** when the selected runtime is registry-backed, the provider catalog + selector models come from `registry_providers`/`registry_models`, and `billingModeForSelectedProvider(provider, catalog)` reads the DERIVED provider's `billing_mode` off the registry catalog. The hardcoded `billingModeForProvider` (`'' | 'platform'` → platform_managed else byok) stays as the fallback only. So the billing-mode the UI shows/sends reflects the DERIVED provider (folds in the closed #1931's canvas intent). ### Constraints - **Federation/back-compat preserved**: a non-registry runtime (external/mock/kimi/future third-party) or an older backend yields `registry_backed=false` → the canvas keeps the template-served models + its heuristic, unchanged. - **NO hard-reject** — the canvas just can't render an option the registry didn't serve for registry-backed runtimes. - **Out of scope (per brief):** the manifest runtime allowlist (`SUPPORTED_RUNTIME_VALUES` / `FALLBACK_RUNTIME_OPTIONS`) is NOT a provider vocabulary and is untouched. `PUT /workspaces/:id/provider` is NOT retired (that CTO-decision-#3 follow-through is a later phase). ### Tests / build (Phase 3) - TDD: `ProviderModelSelector.registry.test.tsx` (catalog bucketed by derived provider, labelled from `display_name`, carries `billing_mode` + `auth_env`, no empty buckets); `ConfigTab.registryBilling.test.tsx` (billing reads registry catalog; falls back to the legacy rule with no catalog / unknown provider). - Full canvas suite green: **223 files, 3380 passed / 1 skipped**. `tsc --noEmit` clean for the touched files (pre-existing unrelated vitest-mock type errors in ContextMenu/EmptyState/OrgCancelButton/SidePanel are untouched). `eslint` 0 issues on touched files (also removed a pre-existing `availableModels` exhaustive-deps warning by memoizing it). ### Five-Axis (Phase 4) Walked all five — Correctness (registry path + heuristic fallback both tested; un-annotated models skipped from the cascade), Readability (no finding), Architecture (heuristic kept as clean federation/back-compat fallback), Security (NAMES-only auth_env, no secrets), Performance (catalog memoized; no new fetch — rides the existing `/templates` call). Approve. cross-ref: internal#718 P3. Stacked on PR-A #1977; PR-C = controlplane provisioner (#2). --- ### SOP checklist (RFC#351 — peer `/sop-ack` to satisfy each) - **Comprehensive testing performed:** 2 new vitest files — `ProviderModelSelector.registry.test.tsx` (catalog bucketed by DERIVED provider, labelled from registry display_name, carries billing_mode + auth_env, no empty buckets) and `ConfigTab.registryBilling.test.tsx` (billing reads the registry catalog; falls back to the legacy rule with no catalog / unknown provider). Full canvas suite green: 223 files, 3380 passed / 1 skipped. Edge cases: non-registry runtime / older backend (registry_backed=false → heuristic fallback), stale saved provider not in catalog (legacy-rule fallback). - **Local-postgres E2E run:** N/A — pure-frontend change (canvas TSX only; no server/DB code path). - **Staging-smoke verified or pending:** scheduled post-merge — after deploy, open a claude-code workspace's Config tab and confirm the Provider dropdown labels + billing-mode come from the registry payload (not VENDOR_LABELS), and a non-registry runtime still renders via the heuristic. - **Root-cause not symptom:** the canvas re-derived provider knowledge client-side (inferVendor/VENDOR_LABELS/BARE_VENDOR_PATTERNS + a hardcoded billingModeForProvider rule) instead of consuming the registry SSOT; this consumes the registry-served /templates fields so the provider vocab + billing-mode come from the DERIVED provider. - **Five-Axis review walked:** Correctness (registry path + heuristic fallback both tested; un-annotated models skipped from the cascade), Readability (no finding), Architecture (heuristic retained as a clean federation/back-compat fallback, not duplicated logic), Security (NAMES-only auth_env in the catalog; no secret values), Performance (catalog memoised; no new fetch — rides the existing /templates call). Approve. - **No backwards-compat shim / dead code added:** No — the inferVendor/buildProviderCatalog heuristic is RETAINED ON PURPOSE as the fallback for non-registry runtimes + older backends (federation/back-compat, not a shim). No dead code; the selector's new optional `catalog` prop is consumed by ConfigTab. - **Memory/saved-feedback consulted:** `feedback_canvas_only_reporting_default` (UI-surface change), `feedback_pr_merge_conventions` (not merged; merge-commit on go), `reference_providers_runtime_matrix_ssot` (canvas now consumes the SSOT-derived list). Removed a pre-existing `availableModels` exhaustive-deps eslint warning while touching the file.

hongming added 1 commit 2026-05-28 02:16:26 +00:00

feat(canvas): P3 internal#718 — consume registry-served /templates list, retire hardcoded provider vocab (#4/#5) ... 120e28ffb5

P3 item 2. The canvas Provider/Model selector + Config-tab billing-mode now
consume the registry-served GET /templates fields (registry_backed /
registry_providers / registry_models from PR-A) instead of re-deriving provider
knowledge client-side. Retires the hardcoded vocabularies as the PRIMARY path:

- ProviderModelSelector (#4): new buildProviderCatalogFromRegistry(providers,
  models) builds the dropdown catalog from the registry payload — provider
  label = registry display_name, bucket = DERIVED provider, billing + auth_env
  from the registry — instead of inferVendor / VENDOR_LABELS /
  BARE_VENDOR_PATTERNS. The selector takes an optional pre-built `catalog`
  prop and uses it verbatim when supplied. inferVendor/buildProviderCatalog
  remain ONLY as the fallback for non-registry runtimes / older backends.
- ConfigTab (#5): when the selected runtime is registry-backed, the provider
  catalog + selector models come from registry_providers/registry_models, and
  billingModeForSelectedProvider(provider, catalog) reads the DERIVED provider's
  billing_mode off the registry catalog. The hardcoded billingModeForProvider
  ('' | 'platform' → platform_managed else byok) stays as the fallback only.
  So the billing-mode the UI shows/sends reflects the DERIVED provider
  (folds in the closed #1931's canvas intent).

Federation/back-compat preserved: a non-registry runtime (external/mock/kimi/
future third-party) or an older backend that doesn't serve the registry fields
yields registry_backed=false → the canvas keeps the template-served models +
its heuristic, unchanged. NO hard-reject (the canvas just can't render an
option the registry didn't serve for registry-backed runtimes).

Out of scope (per brief): the manifest runtime allowlist
(SUPPORTED_RUNTIME_VALUES / FALLBACK_RUNTIME_OPTIONS) is NOT a provider
vocabulary and is untouched; PUT /workspaces/:id/provider is NOT retired (that
CTO #3 follow-through is a later phase).

Stacked on PR-A (workspace-server registry-served /templates); re-target to
main after PR-A merges.

TDD: ProviderModelSelector.registry.test.tsx (catalog bucketed by derived
provider, labelled from display_name, carries billing_mode + auth_env, no empty
buckets), ConfigTab.registryBilling.test.tsx (billing reads registry catalog;
falls back to the legacy rule with no catalog / unknown provider). Full canvas
suite green (3380 passed / 1 skipped), tsc clean for touched files, eslint 0.

internal#718 P3 — not merged; CTO merge-go after Five-Axis (UI-affecting).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

hongming added the tier:medium label 2026-05-28 02:16:44 +00:00

hongming force-pushed feat/internal-718-p3b-canvas-consume-registry from 120e28ffb5 to f2d7f1daf5 2026-05-28 02:21:29 +00:00 Compare

agent-reviewer requested changes 2026-05-28 02:31:42 +00:00

Dismissed

agent-reviewer left a comment

Member

Copy Link

Copy Source

Independent dev-SOP Five-Axis review — PR #1978 (P3 PR-B, canvas consume registry)

Reviewed as agent-reviewer (author hongming, distinct identity). Stacked on #1977 (base = feat/internal-718-p3a-templates-from-registry); diff = canvas delta only (4 files, +388/−16). Verified the PR-A contract on the same branch.

Verdict: REQUEST_CHANGES — one narrow but load-bearing test-discrimination gap on retire-list #5. Everything else passes. The fix is ~1 test + replacing 1 non-discriminating assertion.

Axis-by-axis

1. Registry primary / heuristic fallback-only — PASS. inferVendor/VENDOR_LABELS/BARE_VENDOR_PATTERNS are referenced ONLY inside buildProviderCatalog (ProviderModelSelector.tsx:245-265), which is now reached solely via the fallback branch catalogProp ?? buildProviderCatalog(models) (ProviderModelSelector.tsx:394) and registryBacked ? buildProviderCatalogFromRegistry(...) : buildProviderCatalog(...) (ConfigTab.tsx:642-650). Not dead, not primary. Same for billingModeForSelectedProvider (ConfigTab.tsx:354-364): reads entry.billingMode off the registry catalog first, falls back to billingModeForProvider only when no catalog / provider not carried. Save path now calls billingModeForSelectedProvider(...) (ConfigTab.tsx:855-857). Contract matches PR-A exactly (registry_backed/registry_providers/registry_models; registryProviderView{name,display_name,auth_env,billing_mode,deprecated}; modelSpec{id,name,provider,billing_mode}).

2. No UI regression — PASS (by construction). The selector render path operates on ONE unified catalog variable (ProviderModelSelector.tsx:392); buildProviderCatalogFromRegistry emits the identical ProviderEntry shape (id/vendor/label/envVars/models/wildcard + optional billingMode), so selected/userPickedCustom/handleProviderChange/handleModelChange/dropdown all run identically regardless of catalog source. No parallel render path. The existing ConfigTab.billingMode.test.tsx integration tests (which serve /templates → [] → registryBacked=false) exercise the full Save→PUT flow through the new billingModeForSelectedProvider and still produce byte-identical billing PUTs — strong proof the fallback composition didn't regress.

3. Graceful when registry fields absent — PASS. PR-A emits registry_backed,omitempty (false omitted from wire); canvas reads r.registry_backed === true && registryModels.length > 0 (ConfigTab.tsx:594) — strict, so omitted/old-backend → registryBacked=false → heuristic + template models, no crash/blank. Un-annotated registry model (provider missing, e.g. DeriveProvider overlap) is continue-skipped from the cascade (ProviderModelSelector.tsx) matching PR-A's "serve id without annotation" fail-open. Federation runtimes (external/mock/kimi) hit PR-A's early-return → registryBacked=false. Selector-models scoring (registryBacked?1000:0)+models.length keeps the richer entry.

4. No scope creep — PASS. Purely canvas consumption: all 4 files under canvas/src/components/. No backend change (PR-A owns that), no TS-codegen SDK (API-served, the chosen P3 approach). SUPPORTED_RUNTIME_VALUES/FALLBACK_RUNTIME_OPTIONS untouched as vocab; PUT .../provider not retired (correctly deferred).

5. Quality — PASS w/ note. No console.* in diff. availableModels correctly memoized (ConfigTab.tsx:623) to stabilize identity for the downstream useMemo deps (also clears the pre-existing exhaustive-deps warning). billingMode UI reflects the DERIVED provider via the registry catalog. (CI combined-state is still pending at head — that's a separate merge precondition, not part of this Five-Axis verdict; not merging regardless.)

The blocking finding (Axis-1 / Correctness, retire-list #5 proof)

ConfigTab.registryBilling.test.tsx does not contain a test that would FAIL if billingModeForSelectedProvider regressed to the hardcoded rule for the catalog-hit branch:

• expect(billingModeForSelectedProvider("platform", catalog)).toBe("platform_managed") — hardcoded rule ALSO returns platform_managed for "platform". Non-discriminating.
• expect(billingModeForSelectedProvider("anthropic-oauth", catalog)).toBe("byok") — the test's own comment claims to "use a case the hardcoded rule gets WRONG to prove the registry is authoritative," but anthropic-oauth is non-empty/non-platform so the hardcoded rule ALSO returns byok. The assertion passes against BOTH the registry-authoritative impl and a regression to the hardcoded rule. Tautological + misleading comment.

The heart of retire-list #5 ("billing reflects the DERIVED provider per the registry, not the hardcoded '' | 'platform' → platform_managed else byok rule") is therefore not pinned. buildProviderCatalogFromRegistry carrying billing_mode verbatim IS discriminatingly tested in ProviderModelSelector.registry.test.tsx (good), but the wrapper's "registry value wins over a disagreeing hardcoded result" branch is not.

Requested change (small): add a disagreement case where the registry says one thing and the hardcoded rule says the other — i.e. a registry provider with a non-empty/non-platform NAME but billing_mode: "platform_managed":

const DISAGREE: RegistryProvider[] = [
  { name: "anthropic-api", display_name: "Anthropic API", billing_mode: "platform_managed" },
];
const DISAGREE_MODELS: RegistryModel[] = [
  { id: "claude-x", provider: "anthropic-api", billing_mode: "platform_managed" },
];
const c = buildProviderCatalogFromRegistry(DISAGREE, DISAGREE_MODELS);
// hardcoded rule says byok for "anthropic-api"; registry says platform_managed → registry must win
expect(billingModeForSelectedProvider("anthropic-api", c)).toBe("platform_managed");

Replace (or keep but de-tautologize the comment of) the current anthropic-oauth → byok assertion, since it doesn't discriminate. Optionally a thin integration test driving Save with a registry-backed runtime whose derived provider's registry billing disagrees with the hardcoded rule, asserting the mode: in the /llm-billing-mode PUT body comes from the registry — but the unit-level disagreement case above is the minimum to close the gate.

Everything else is ready; this is the only blocker. Re-request review once the discriminating test lands.

## Independent dev-SOP Five-Axis review — PR #1978 (P3 PR-B, canvas consume registry) Reviewed as `agent-reviewer` (author `hongming`, distinct identity). Stacked on #1977 (base = `feat/internal-718-p3a-templates-from-registry`); diff = canvas delta only (4 files, +388/−16). Verified the PR-A contract on the same branch. **Verdict: REQUEST_CHANGES** — one narrow but load-bearing test-discrimination gap on retire-list #5. Everything else passes. The fix is ~1 test + replacing 1 non-discriminating assertion. ### Axis-by-axis 1. **Registry primary / heuristic fallback-only — PASS.** `inferVendor`/`VENDOR_LABELS`/`BARE_VENDOR_PATTERNS` are referenced ONLY inside `buildProviderCatalog` (ProviderModelSelector.tsx:245-265), which is now reached solely via the fallback branch `catalogProp ?? buildProviderCatalog(models)` (ProviderModelSelector.tsx:394) and `registryBacked ? buildProviderCatalogFromRegistry(...) : buildProviderCatalog(...)` (ConfigTab.tsx:642-650). Not dead, not primary. Same for `billingModeForSelectedProvider` (ConfigTab.tsx:354-364): reads `entry.billingMode` off the registry catalog first, falls back to `billingModeForProvider` only when no catalog / provider not carried. Save path now calls `billingModeForSelectedProvider(...)` (ConfigTab.tsx:855-857). Contract matches PR-A exactly (`registry_backed`/`registry_providers`/`registry_models`; `registryProviderView{name,display_name,auth_env,billing_mode,deprecated}`; `modelSpec{id,name,provider,billing_mode}`). 2. **No UI regression — PASS (by construction).** The selector render path operates on ONE unified `catalog` variable (ProviderModelSelector.tsx:392); `buildProviderCatalogFromRegistry` emits the identical `ProviderEntry` shape (id/vendor/label/envVars/models/wildcard + optional billingMode), so `selected`/`userPickedCustom`/`handleProviderChange`/`handleModelChange`/dropdown all run identically regardless of catalog source. No parallel render path. The existing `ConfigTab.billingMode.test.tsx` integration tests (which serve `/templates → []` → registryBacked=false) exercise the full Save→PUT flow through the new `billingModeForSelectedProvider` and still produce byte-identical billing PUTs — strong proof the fallback composition didn't regress. 3. **Graceful when registry fields absent — PASS.** PR-A emits `registry_backed,omitempty` (false omitted from wire); canvas reads `r.registry_backed === true && registryModels.length > 0` (ConfigTab.tsx:594) — strict, so omitted/old-backend → registryBacked=false → heuristic + template models, no crash/blank. Un-annotated registry model (`provider` missing, e.g. DeriveProvider overlap) is `continue`-skipped from the cascade (ProviderModelSelector.tsx) matching PR-A's "serve id without annotation" fail-open. Federation runtimes (external/mock/kimi) hit PR-A's early-return → registryBacked=false. Selector-models scoring `(registryBacked?1000:0)+models.length` keeps the richer entry. 4. **No scope creep — PASS.** Purely canvas consumption: all 4 files under `canvas/src/components/`. No backend change (PR-A owns that), no TS-codegen SDK (API-served, the chosen P3 approach). `SUPPORTED_RUNTIME_VALUES`/`FALLBACK_RUNTIME_OPTIONS` untouched as vocab; `PUT .../provider` not retired (correctly deferred). 5. **Quality — PASS w/ note.** No `console.*` in diff. `availableModels` correctly memoized (ConfigTab.tsx:623) to stabilize identity for the downstream useMemo deps (also clears the pre-existing exhaustive-deps warning). billingMode UI reflects the DERIVED provider via the registry catalog. (CI combined-state is still `pending` at head — that's a separate merge precondition, not part of this Five-Axis verdict; not merging regardless.) ### The blocking finding (Axis-1 / Correctness, retire-list #5 proof) `ConfigTab.registryBilling.test.tsx` does not contain a test that would FAIL if `billingModeForSelectedProvider` regressed to the hardcoded rule for the catalog-hit branch: - `expect(billingModeForSelectedProvider("platform", catalog)).toBe("platform_managed")` — hardcoded rule ALSO returns platform_managed for `"platform"`. Non-discriminating. - `expect(billingModeForSelectedProvider("anthropic-oauth", catalog)).toBe("byok")` — the test's own comment claims to "use a case the hardcoded rule gets WRONG to prove the registry is authoritative," but `anthropic-oauth` is non-empty/non-platform so the hardcoded rule ALSO returns byok. The assertion passes against BOTH the registry-authoritative impl and a regression to the hardcoded rule. Tautological + misleading comment. The heart of retire-list #5 ("billing reflects the DERIVED provider per the registry, not the hardcoded `'' | 'platform' → platform_managed else byok` rule") is therefore not pinned. `buildProviderCatalogFromRegistry` carrying `billing_mode` verbatim IS discriminatingly tested in `ProviderModelSelector.registry.test.tsx` (good), but the wrapper's "registry value wins over a disagreeing hardcoded result" branch is not. **Requested change (small):** add a disagreement case where the registry says one thing and the hardcoded rule says the other — i.e. a registry provider with a non-empty/non-`platform` NAME but `billing_mode: "platform_managed"`: ```ts const DISAGREE: RegistryProvider[] = [ { name: "anthropic-api", display_name: "Anthropic API", billing_mode: "platform_managed" }, ]; const DISAGREE_MODELS: RegistryModel[] = [ { id: "claude-x", provider: "anthropic-api", billing_mode: "platform_managed" }, ]; const c = buildProviderCatalogFromRegistry(DISAGREE, DISAGREE_MODELS); // hardcoded rule says byok for "anthropic-api"; registry says platform_managed → registry must win expect(billingModeForSelectedProvider("anthropic-api", c)).toBe("platform_managed"); ``` Replace (or keep but de-tautologize the comment of) the current `anthropic-oauth → byok` assertion, since it doesn't discriminate. Optionally a thin integration test driving Save with a registry-backed runtime whose derived provider's registry billing disagrees with the hardcoded rule, asserting the `mode:` in the `/llm-billing-mode` PUT body comes from the registry — but the unit-level disagreement case above is the minimum to close the gate. Everything else is ready; this is the only blocker. Re-request review once the discriminating test lands.

core-be referenced this issue from a commit 2026-05-28 02:43:17 +00:00

test(canvas): make registryBilling test discriminate registry-vs-hardcoded billing precedence (#1978 review)

core-be added 1 commit 2026-05-28 02:43:17 +00:00

test(canvas): make registryBilling test discriminate registry-vs-hardcoded billing precedence (#1978 review) ... 3f8c4e7f8d

agent-reviewer #7790 (blocking) found that ConfigTab.registryBilling.test.tsx
did not actually pin retire-list #5's core claim — both existing assertions
("platform"→platform_managed, "anthropic-oauth"→byok) return the SAME value
under both the registry-authoritative impl and a regression to the old
hardcoded billingModeForProvider rule, so the test was tautological and a
regression would still pass. The misleading comment on the anthropic-oauth
case claimed it was "a case the hardcoded rule gets WRONG" but the hardcoded
rule actually agrees there too.

This commit adds a genuine disagreement case: a registry provider
"managed-federated" whose registry-served billing_mode is "platform_managed"
even though its name is not "" / "platform" (so the legacy
billingModeForProvider rule would return "byok"). The new test asserts the
two rules disagree on this input (sanity) and then asserts
billingModeForSelectedProvider returns the REGISTRY value
("platform_managed"), which is only reachable by honoring the catalog.

Load-bearing proof: with the registry-first impl, the new test PASSES; when
billingModeForSelectedProvider is temporarily forced to fall through to the
hardcoded rule, the new test (and only the new test) FAILS with
expected 'platform_managed' / received 'byok' — proving it pins the
registry-wins contract.

Also fixes the misleading "hardcoded rule gets WRONG" comment on the
anthropic-oauth case (explicitly annotates it as non-discriminating and
points to the new disagreement case as the registry-WINS proof).

Implementation (billingModeForSelectedProvider) untouched — confirmed
byte-identical to PR #1978 HEAD (f2d7f1da).

Verification:
  - targeted: 5 passed (was 4 — adds the discriminating case)
  - regressed-impl: only the new test fails, others pass (= they are
    non-discriminating as the review found)
  - full canvas vitest: 223 files / 3381 passed | 1 skipped (3382) — +1
    vs the 3380/1 baseline
  - tsc: 0 new errors (touched file clean; pre-existing 223 baseline
    unchanged with my diff stashed)
  - eslint on touched file: 0

Refs: #1978, review #7790, internal#718 P3 retire-list #5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

hongming commented 2026-05-28 02:43:48 +00:00

Author

Owner

Copy Link

Copy Source

Addressed agent-reviewer #7790 (blocking) with a test-only fix at 3f8c4e7f — implementation is unchanged (byte-identical to f2d7f1da).

What the review found: the two existing assertions in ConfigTab.registryBilling.test.tsx ("platform" → platform_managed, "anthropic-oauth" → byok) return the same value under both the registry-authoritative impl AND a regression to the old hardcoded billingModeForProvider rule — tautological. The anthropic-oauth comment even claimed it was "a case the hardcoded rule gets WRONG" but the rules actually agree on that input. So nothing in the test pinned the registry-wins contract.

Fix (test-only):

1. Added a discriminating fixture: a registry provider managed-federated whose registry-served billing_mode is platform_managed even though its name isn’t "" / "platform" (so the legacy billingModeForProvider rule would say byok). This is the federation-ready shape the SSOT is designed for.
2. Added a new test "lets the registry billing_mode WIN when it disagrees with the hardcoded rule" that first asserts the two rules genuinely disagree on this input (sanity), then asserts billingModeForSelectedProvider("managed-federated", catalog) returns "platform_managed" — reachable only by honoring the catalog.
3. De-tautologized the misleading anthropic-oauth comment (annotates it as non-discriminating and points to the new disagreement case as the registry-WINS proof).

Load-bearing proof (test does what the title claims):

• Current registry-first impl: 5 passed (new test included).
• Regressed impl (temporarily forced billingModeForSelectedProvider to fall straight through to billingModeForProvider): only the new discriminating test fails (expected 'byok' to be 'platform_managed'); the other 4 still pass — confirming the review’s finding that they’re non-discriminating. The impl was reverted immediately; git diff canvas/src/components/tabs/ConfigTab.tsx is empty.

Verification:

• Full canvas vitest: 223 files / 3381 passed | 1 skipped (3382) — +1 vs the 3380/1 baseline (the discriminating case).
• tsc on touched file: 0 errors; total error count unchanged at 223 with my change vs stashed (pre-existing baseline in unrelated test files).
• eslint on touched file: 0.

No implementation change — the review explicitly noted the impl is correct and passed all five axes; this PR’s billing-precedence behavior is intact. Not requesting merge.

Addressed agent-reviewer #7790 (blocking) with a test-only fix at `3f8c4e7f` — implementation is unchanged (byte-identical to f2d7f1da). **What the review found:** the two existing assertions in `ConfigTab.registryBilling.test.tsx` (`"platform" → platform_managed`, `"anthropic-oauth" → byok`) return the same value under both the registry-authoritative impl AND a regression to the old hardcoded `billingModeForProvider` rule — tautological. The `anthropic-oauth` comment even claimed it was "a case the hardcoded rule gets WRONG" but the rules actually agree on that input. So nothing in the test pinned the registry-wins contract. **Fix (test-only):** 1. Added a discriminating fixture: a registry provider `managed-federated` whose registry-served `billing_mode` is `platform_managed` even though its name isn’t `""` / `"platform"` (so the legacy `billingModeForProvider` rule would say `byok`). This is the federation-ready shape the SSOT is designed for. 2. Added a new test `"lets the registry billing_mode WIN when it disagrees with the hardcoded rule"` that first asserts the two rules genuinely disagree on this input (sanity), then asserts `billingModeForSelectedProvider("managed-federated", catalog)` returns `"platform_managed"` — reachable only by honoring the catalog. 3. De-tautologized the misleading `anthropic-oauth` comment (annotates it as non-discriminating and points to the new disagreement case as the registry-WINS proof). **Load-bearing proof (test does what the title claims):** - Current registry-first impl: 5 passed (new test included). - Regressed impl (temporarily forced `billingModeForSelectedProvider` to fall straight through to `billingModeForProvider`): only the new discriminating test fails (`expected 'byok' to be 'platform_managed'`); the other 4 still pass — confirming the review’s finding that they’re non-discriminating. The impl was reverted immediately; `git diff canvas/src/components/tabs/ConfigTab.tsx` is empty. **Verification:** - Full canvas vitest: **223 files / 3381 passed | 1 skipped (3382)** — +1 vs the 3380/1 baseline (the discriminating case). - tsc on touched file: 0 errors; total error count unchanged at 223 with my change vs stashed (pre-existing baseline in unrelated test files). - eslint on touched file: 0. No implementation change — the review explicitly noted the impl is correct and passed all five axes; this PR’s billing-precedence behavior is intact. Not requesting merge.

agent-reviewer approved these changes 2026-05-28 02:57:59 +00:00

Dismissed

agent-reviewer left a comment

Member

Copy Link

Copy Source

re-approve after test-tautology fix on head 3f8c4e7f; registryBilling test now discriminates registry-vs-hardcoded billing precedence via the managed-federated disagreement case (mutation-proven), impl unchanged, full canvas suite +1 baseline-green

re-approve after test-tautology fix on head 3f8c4e7f; registryBilling test now discriminates registry-vs-hardcoded billing precedence via the managed-federated disagreement case (mutation-proven), impl unchanged, full canvas suite +1 baseline-green

hongming changed target branch from feat/internal-718-p3a-templates-from-registry to main 2026-05-28 03:03:26 +00:00

agent-reviewer approved these changes 2026-05-28 03:03:27 +00:00

Dismissed

agent-reviewer left a comment

Member

Copy Link

Copy Source

approve on current head after retarget (P3 PR-B canvas, identical-content)

approve on current head after retarget (P3 PR-B canvas, identical-content)

claude-ceo-assistant approved these changes 2026-05-28 03:03:27 +00:00

Dismissed

claude-ceo-assistant left a comment

Owner

Copy Link

Copy Source

approve on current head after retarget

approve on current head after retarget

hongming referenced this issue from a commit 2026-05-28 03:42:48 +00:00

test(canvas): make registryBilling test discriminate registry-vs-hardcoded billing precedence (#1978 review)

hongming force-pushed feat/internal-718-p3b-canvas-consume-registry from 3f8c4e7f8d to 2e3f4b3462 2026-05-28 03:42:48 +00:00 Compare

agent-reviewer approved these changes 2026-05-28 03:43:13 +00:00

Dismissed

agent-reviewer left a comment

Member

Copy Link

Copy Source

approve on rebased head — identical canvas-consume-registry delta

approve on rebased head — identical canvas-consume-registry delta

claude-ceo-assistant approved these changes 2026-05-28 03:43:14 +00:00

Dismissed

claude-ceo-assistant left a comment

Owner

Copy Link

Copy Source

approve on rebased head

approve on rebased head

claude-ceo-assistant approved these changes 2026-05-28 04:19:57 +00:00

Dismissed

claude-ceo-assistant left a comment

Owner

Copy Link

Copy Source

Re-approving on new head after rebase onto main post-#1981 merge (conflict resolved). All prior review concerns remain addressed; no new behavior changes in the rebase.

Re-approving on new head after rebase onto main post-#1981 merge (conflict resolved). All prior review concerns remain addressed; no new behavior changes in the rebase.

hongming referenced this issue from a commit 2026-05-28 04:19:58 +00:00

test(canvas): make registryBilling test discriminate registry-vs-hardcoded billing precedence (#1978 review)

hongming force-pushed feat/internal-718-p3b-canvas-consume-registry from 2e3f4b3462 to 213e4cef4d 2026-05-28 04:19:58 +00:00 Compare

agent-reviewer approved these changes 2026-05-28 04:19:58 +00:00

Dismissed

agent-reviewer left a comment

Member

Copy Link

Copy Source

Re-approving on rebased head 213e4cef4d: rebase onto main was clean; PR-B's canvas-consumes-registry delta is unchanged. Five-Axis previously satisfied (registry-wins discrimination test load-bearing). Approve to merge.

Re-approving on rebased head 213e4cef4dbe9a46afd295327a581335df83df4a: rebase onto main was clean; PR-B's canvas-consumes-registry delta is unchanged. Five-Axis previously satisfied (registry-wins discrimination test load-bearing). Approve to merge.

claude-ceo-assistant approved these changes 2026-05-28 05:06:41 +00:00

Dismissed

claude-ceo-assistant left a comment

Owner

Copy Link

Copy Source

Re-approving on rebased head 8546502ab8. Manual conflict resolution on ConfigTab.tsx: took HEAD (post-#1984 retirement of canvas-side billing-mode PUT). Server now derives billing-mode from (runtime, model) via registry; canvas push is redundant. PR-Bs registry-READ value (buildProviderCatalogFromRegistry + registry_* destructuring + billingModeForSelectedProvider helper for display) preserved elsewhere in the file (6 refs). Both new test files intact.

Re-approving on rebased head 8546502ab8819e2a9fdffd631d0621c25b009dab. Manual conflict resolution on ConfigTab.tsx: took HEAD (post-#1984 retirement of canvas-side billing-mode PUT). Server now derives billing-mode from (runtime, model) via registry; canvas push is redundant. PR-Bs registry-READ value (buildProviderCatalogFromRegistry + registry_* destructuring + billingModeForSelectedProvider helper for display) preserved elsewhere in the file (6 refs). Both new test files intact.

agent-reviewer approved these changes 2026-05-28 05:06:41 +00:00

Dismissed

agent-reviewer left a comment

Member

Copy Link

Copy Source

Re-approving on rebased head 8546502ab8. Conflict resolution verified: HEAD retains #1984s retirement of the canvas-side billing-mode write path. PR-Bs READ-side registry-consume survives intact (6 refs across helper definitions + provider catalog hook + type destructuring). Approve to merge.

Re-approving on rebased head 8546502ab8819e2a9fdffd631d0621c25b009dab. Conflict resolution verified: HEAD retains #1984s retirement of the canvas-side billing-mode write path. PR-Bs READ-side registry-consume survives intact (6 refs across helper definitions + provider catalog hook + type destructuring). Approve to merge.

hongming referenced this issue from a commit 2026-05-28 05:06:43 +00:00

test(canvas): make registryBilling test discriminate registry-vs-hardcoded billing precedence (#1978 review)

hongming force-pushed feat/internal-718-p3b-canvas-consume-registry from 213e4cef4d to 8546502ab8 2026-05-28 05:06:43 +00:00 Compare

hongming dismissed claude-ceo-assistant's review 2026-05-28 05:06:43 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

hongming dismissed agent-reviewer's review 2026-05-28 05:06:43 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

hongming merged commit 03aa69f46f into main 2026-05-28 05:59:08 +00:00

hongming referenced this issue from a commit 2026-05-28 05:59:09 +00:00

Merge pull request 'P3 internal#718: canvas consumes registry-served /templates, retire hardcoded provider vocab #4/#5 (PR-B; NOT merged)' (#1978) from feat/internal-718-p3b-canvas-consume-registry into main

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label tier:medium

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

4 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1978