[ggbdpq]

Summary

Regular chat supplies renderProviderMark to the shared Composer, so its model switcher shows each provider's brand mark; WorkHub never passed it, so the same configured models lost their visual identifier on the WorkHub surface (the compact wheel stays as #5444 designed).

The provider brand marks now live in the connection-settings feature beside GenericProviderMark, which already owns provider UI. The composition root (composition/legacy-desktop-region.tsx) takes that public export and injects it into both the legacy shell's composer and WorkHubRoot, so the renderer entry (app.tsx) stays exactly as it was: no new import and no change to its permanently guarded entry budget. app-shell.tsx and the two settings consumers read the same public export.

The vendored brand SVGs are referenced with new URL(..., import.meta.url) instead of static imports because eight src/main/__tests__ files import the connection-settings barrel and the main tsc graph has no SVG loader. Vite resolves the same asset URLs at renderer build time, and the existing CSS-mask rendering keeps currentColor theming for the monochrome marks.

renderer-architecture.json is regenerated with --write: the legacy file set and the AppShell closure lose the moved module, the priced app.tsx import is gone, and the new public export is admitted as a root-symbol use of this same change. The strict-base comparison against fresh upstream/main passes and is a net debt reduction.

A WorkHubComposer render test pins the mark beside the model switcher, and a source-wiring test pins both hops from the composition adapter to the composer.

Verification

Check	Result
check:architecture --base upstream/main --strict-base	pass; app.tsx budget unchanged (5 import declarations / 6 specifiers / 172 tokens)
Architecture checker self-tests	168 pass / 0 fail
New render + source-wiring tests	2 pass / 0 fail
Affected main tests (6 files, incl. the connection-settings barrel consumers)	34 pass / 0 fail
build:renderer (Vite resolution of the new URL assets)	pass
biome check --write on the changed files	clean
check:asf-headers	pass

AI use

The original commits carry a Generated-by: GLM-5.3-Flash (ZCode) trailer; the zero-debt rewiring commit carries Generated-by: DeepSeek Harness (dsh). This PR description was written with AI assistance.

Checklist

• No copy changes (no locale keys touched)
• No new dependencies
• Compact WorkHub wheel untouched (feat(ui): restore list pickers for model and thinking selection #5444 scope preserved)
• Renderer entry budget unchanged; strict-base passes against fresh upstream/main
• Visual consistency on the live WorkHub surface still merits a maintainer spot-check

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

At fd51dc5c8e0634971668e0ee46ede159a6a6b8da, one P2 issue prevents the current change from passing CI: apps/desktop/src/renderer/app.tsx:25 adds a dependency and increases the permanently guarded renderer-entry budget. The strict check against the actual base reports dependency count 5→6, import declarations 5→6, import specifiers 6→7, a new dependency on ./settings/provider-brand-marks, and token count 172→196. Regenerating the current ledger does not authorize those increases. I reproduced the same five failures locally; the current CI test run fails there, while the base's CI run passes the architecture check. Preserve the existing entry budget and rerun the strict comparison, rather than relying on the plain regenerated-ledger check.

The provider-icon premise is supported by the production call chain and a browser reproduction, not merely the issue description. Regular chat already supplies the existing ProviderBrandMark implementation; the previous WorkHubRoot omits it. The new callback reaches the existing WorkHubComposer without adding another model-selection authority or new icon assets. Six real-Chromium runs using the existing WorkHubRoot stories, with the same provider renderer supplied as in production, pass for docked, expanded-conversation and progress-composer states in light and dark themes. The existing OpenAI SVG paints at approximately 16 px with the theme's foreground color. Replacing only WorkHubRoot with the exact base implementation causes the docked model-menu icon assertion to fail after the story's interaction assertions complete. These runs exercise the real controller/root and existing story services, not the Electron App entry or a live provider.

In the unified menu, the icons appear on model options, not on the menu trigger. The injected callback also supplies an icon to the compact wheel's trigger; the wheel options and selection behavior are unchanged. The new component test alone does not verify root wiring because WorkHubComposer already forwarded this prop before the PR.

The normal build, four Desktop type configurations, production renderer build, all 168 WorkHub tests, inventory and formatting pass. No additional verified P0–P3 issue was found.

There are no protocol changes against the author's base, which remains at epoch 220. Main advanced to abb2deca during validation; its guarded App entry and the reviewed wiring are unchanged. The source merge is clean and retains main's epoch 222, but that merged result was not built or tested. GitHub reports the PR as mergeable with its test check failed and the overall state BLOCKED. Any feature approval remains a maintainer decision after the CI failure is fixed.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review from fd51dc5c to 0c855867721f8412ae6ab7febd4217342d93d9fa. The new commit makes renderProviderMark required on AppShell and WorkHubRoot, which have one caller each. It also adds a source-wiring test that pins both hops from the composition adapter to the composer. Both changes look correct, and I found no new issue.

Still open: P2, the app.tsx entry budget. The test job on this head still fails at check-renderer-architecture.mjs --base 8686793e… --strict-base. That is the same failure as before. The commit message explains why the author sees no legal alternative supplier and asks for a maintainer ruling on the budget increase. This PR cannot merge until a maintainer either accepts the repricing or chooses a different wiring and CI turns green.

[ggbdpq]

@liugddx gentle ping — this PR has been carrying an open architecture ruling since Oct 14 (thread PRRT_kwDOSpfFGs6sQWmR): the strict-base failure on app.tsx's entry budget cannot be resolved in code. We verified the checker leaves no legal zero-debt supplier for the WorkHub provider mark — composition cannot import legacy, the feature-declarable edge would drag 20+ static svg imports into the svg-loader-less main test graph, and app-shell.tsx itself sits under the same legacyAppShell.files ratchet. The direction is yours: accept the minimal entry-budget increase (+1 import edge, 172→196 tokens), or we ablate the WorkHub mark to keep the ratchet frozen. Happy to execute either.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review from fd51dc5c8e0634971668e0ee46ede159a6a6b8da to 4f87090724acb417b325cac14dbe09b24364ff98. The author added five commits (0c85586, ef69369, 324c1d7, f727a2d, 27e34bc) and merged main twice; the head's merge base is current main 57cde86b. I separated main's changes from the author's by comparing the old PR diff (against ecd4dba8) with the new PR diff (against 57cde86b).

Prior findings:

• P2, renderer-entry budget / CI failure: fixed. apps/desktop/src/renderer/app.tsx is no longer changed. The provider mark is now wired only in composition/legacy-desktop-region.tsx. provider-brand-marks.tsx moves into features/connection-settings and is exported from that barrel, so app-shell.tsx shrinks (import declarations 34->33, specifiers 43->42). Run locally, check-renderer-architecture.mjs --base 57cde86b --strict-base passes; it only reports, without ratcheting, the new AppShell-closure uses of ProviderBrandMark in general-settings-page and provider-display. CI's strict-base step also passes on this head.
• Prior note that the component test did not cover root wiring: addressed. WorkHubRoot's renderProviderMark prop is now required, so the type checker forces both the composition root and the story to supply it. A second test also pins the forwarding in source.

New changes reviewed:

• The 24 provider SVG imports became new URL('../../assets/...', import.meta.url).href. This matches the existing feature-module convention (external-agent-settings, module-hub). In the production renderer bundle, the small SVGs are still inlined as data URIs and localai.svg (111 KB) is still emitted as a hashed asset, so runtime behavior is unchanged.
• Removing the GenericProviderMark barrel export leaves no dangling importers: typecheck and Knip pass in CI. provider-brand-marks.tsx now imports it directly.
• The Astryx surface inventory and the architecture ledger are updated consistently with the move.

New findings: no P0-P3 issues found.

Validation: on the head, the root npm run build passes, including the production renderer build, the renderer-entry output check and the third-party notices check. Local tsc --noEmit passes for tsconfig.main, renderer and storybook. Of the main-process tests, the 2 new provider-mark tests pass, and all 175 tests across the 21 WorkHub test files pass. Locally, tsconfig.preload reports TS2339 in src/preload/runtime-host-session-catalog.ts, a file this PR does not touch. CI's Typecheck step passes on the same head, so this is a local environment artifact and not PR-caused.

Protocol/epoch: no protocol files are touched; no epoch impact.

CI/mergeability: the test check passes on 4f870907 (run), including strict architecture, Typecheck, Knip, Storybook smoke and the affected workspace tests. The known event-wait-manager Retry-After flake did not occur. GitHub reports the PR as MERGEABLE, with merge state BLOCKED, presumably pending required review. Feature approval remains a maintainer decision.