docs(architecture): audit Model access slice under #3522

[bytelazy]

Summary

Audits the Model access subsystem slice under #3522 — the two documents listed there were read against current main and either corrected or confirmed accurate.

• docs/architecture/openai-responses-incremental-transport.md — verified accurate, no change. Every cited symbol (ModelAdapter, endLane, previous_response_id, prompt_cache_key, the sdk.response.messages contrast) still exists with the semantics the document describes.
• docs/model-metadata-firstscreen-optimization.md — the optimization it proposes has been implemented. The renderer startup path no longer statically imports model-metadata; OnboardingHero no longer imports RECOMMENDED_PROVIDER_TYPES at runtime; PROVIDER_REGISTRY is now reached only from lazy Settings paths — matching the document's desired outcome. The document was being read as an open proposal when its acceptance criteria are met, so it is moved to docs/archive/ with an Archived: banner so it is read as the historical design record.

The move is a git rename (history preserved); the only content change is the two-line banner. docs/archive/** is out of scope for #3522's audit, but moving a now-implemented proposal into the archive is the audit conclusion for this document, not an edit to an existing archive entry.

Refs #3522

Verification

• Confirmed no script, test, or other document references model-metadata-firstscreen-optimization by path, so the move breaks no links (checked scripts/, .github/, all *.md/*.ts/*.mjs).
• Ran node --test scripts/source-legal-inventory.test.mjs (the test that reads docs/): 5/5 pass.
• Confirmed the cited symbols in the accurate document still exist via grep against packages/runtime/src.
• Git recognizes the operation as a rename (R), so blame/history is preserved.

Not run: the repo's full lint/typecheck (a full npm ci does not complete from this network). The change is docs-only and confined to docs/; no code or test is touched.

AI use

• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — audited both documents against current code, implemented the archive move and banner, and authored the commit. The commit carries a Generated-by: Claude (Claude Code) trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

(Both unchecked: this is a docs-only change with no test surface; the relevant source-legal-inventory test passes as noted under Verification.)

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed current head 953a003492cd92fa09ad8f1021cbef07eff125a2 (OPEN). One P2 below — the banner's "implemented, archivable" conclusion does not hold. Note: this is the same first-screen dependency chain flagged on PR #4794; the two should be handled together so the wording stays consistent.

P2 — two of the banner's three factual claims fail, so "implemented, archivable" fails

The move itself is clean (pure git rename, 96% similarity, history preserved, +2 lines). Claim (b) holds: onboarding-hero.tsx:27 is import type only, runtime uses the local FIRST_RUN_PROVIDER_TYPES constant. But (a) and (c) fail — a fully static chain still reaches provider-registry on the first screen: main.tsx:22 → app.tsx:23 → app-shell.tsx:170 → app-shell-overlays.tsx:30 → app-shell-command-actions.ts:36 → command-palette-commands.ts:50 → isRetiredProvider from @maka/core/provider-registry. All six hops are static imports (the dynamic import at app-shell-overlays.tsx:42 leads to settings/settings-modal, a different path, not this chain) — and past provider-registry the dependency is a value dependency, not shakeable side effects.

What I could not judge

Whether the team wants the doc archived with corrected claims vs kept live is a maintainer call.

---

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

简体中文

评审结论来自自动化审查流程；发布者没有读这份 diff，核的是当前 head 有没有漂移。当前 head 是 953a003，未关闭。一条 P2：横幅三条事实有两条不成立，和 #4794 是同一条依赖链，建议两处一起处理。修好再合。

[Astro-Han]

Two corrections to my review above. The P2 itself stands unchanged; both corrections concern how it was stated.

Scope of the evidence. The review omitted a limitation that materially bounds the finding: no build was run and no bundle output was measured. The dependency chain reported above is static import-graph analysis, not byte-level evidence that the first-screen chunk contains the models.dev metadata. Producing that evidence would require a build followed by inspection of the emitted chunk, which was not done. One supporting detail that was checked and does belong in the record: packages/core/package.json declares "sideEffects": false, so a bundler is permitted to drop unused module-scope code — but that does not rescue this chain, because isRetiredProvider has a genuine value dependency on PROVIDER_REGISTRY (provider-registry.ts:1685 → providerDefaultsOf at :1644-1648), which in turn is constructed from the generated models.dev tables imported at :20-24. Tree shaking removes what is unused; it cannot remove a constant that is used.

Merge posture. The Simplified Chinese block ended with wording that amounted to "fix before merging". That was not the reviewer's conclusion and it contradicts the English body of the same comment. This review does not gate the merge. The finding is that the banner's claims (a) and (c) do not hold, and the choice of remedy belongs to the maintainer. Two options were offered: withdraw the archive, keep the document under docs/, and record this surviving chain in its current-state section; or break the chain in code first — for example by extracting the small retirement predicate that isRetiredProvider needs out of provider-registry, so the first screen does not pull in the whole model-metadata table for one boolean — and archive afterwards.

---

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

简体中文

两处更正，P2 结论本身不变，更正的是表述。

其一，上一条漏了一项限定：没有跑构建、没有测量产物体积，那条依赖链是静态引用图分析，不是「首屏 chunk 确实含 models.dev 元数据」的字节级证据。

其二，上一条中文块结尾的「修好再合」不是本次评审的结论，且与同一条评论的英文正文矛盾。本评审不设合并门禁。 结论是横幅的 (a)(c) 两条事实不成立，采取哪种补救由维护者决定；已给出的两个方案是撤回归档并在文档现状小节记录这条残留链路，或先在代码里断链后再归档。

[bytelazy]

Thanks for the P2 — verified, and you're right.

I traced the chain you flagged and it holds: main.tsx → app.tsx → app-shell.tsx:169 (AppShellOverlays) → app-shell-overlays.tsx:30 → app-shell-command-actions.ts:36 → command-palette-commands.ts:50 (value import of isRetiredProvider) → providerDefaultsOf → PROVIDER_REGISTRY ← model-metadata.generated.js (provider-registry.ts:24). PROVIDER_REGISTRY is a used constant, so tree-shaking can't drop it. My original banner's claims (a) and (c) were wrong; I had only grepped direct imports of model-metadata and missed the transitive chain through command-palette-commands.

Updated the PR: dropped the archive move, kept the document live under docs/, and replaced the banner with an audit note that records the surviving chain (what's done, what's not, and why it isn't archivable yet). The doc itself is otherwise unchanged, so the diff is now +2 lines on one file.

Left the code-break-the-chain option (extracting isRetiredProvider's predicate out of provider-registry) for a separate change — that's a code fix, not a docs audit. The note names it as the follow-up that would let this document eventually be archived.

[hqhq1025]

Re-reviewed exact head e260c897c4023b51aed0113c4f9e8276e75195cb. No P0-P3 findings. The single-file change now accurately records the optimization as only partially implemented and keeps the document live instead of archiving it. I independently followed the current static renderer chain from main.tsx through AppShellOverlays and command-palette-commands.ts to the value import of isRetiredProvider, then through providerDefaultsOf to the used PROVIDER_REGISTRY constructed from generated models.dev metadata. I also confirmed that OnboardingHero uses the local FIRST_RUN_PROVIDER_TYPES value and imports only connection/provider types from Core. The OpenAI Responses transport symbols cited by the PR remain present with the documented ownership shape.\n\nVerification: scripts/source-legal-inventory.test.mjs passed 5/5 under Node 24, git diff --check passed, and the exact head merges cleanly with current main (42fa4d070504e672173c46dd75f7fcabb88789f5). GitHub exposes no hosted check runs for this head. I did not build or inspect the emitted renderer bundle, so the conclusion is limited to the static dependency graph rather than measured bundle bytes.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[me2seeks]

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

Summary

Docs-only audit note recording that the first-screen optimization is only partially implemented: the renderer's import chain command-palette-commands.ts:50 still value-imports isRetiredProvider from @maka/core/provider-registry, reaching the generated PROVIDER_REGISTRY constant (provider-registry.ts:1625), which tree-shaking cannot drop. I verified both ends of that claim against the worktree — the value import exists and the registry is a used constant constructed from the models.dev tables, so the note's "not done" assessment is accurate, as is its proposed seam (extract the small retirement predicate out of provider-registry). Honest, precise audit text with no code change.

Findings

No material findings. The doc's own suggestion (extracting the retirement predicate) is the right next step and correctly identified as the minimal break in the chain.

Verdict

merge-ready — accurate audit note; content-only change.

[github-actions]

This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed.

Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the pinned label.

[bytelazy]

Main already records the surviving command-palette → provider-registry dependency in the audit added by #4794 (5ded98e8). This PR now appears redundant; #5018 is the separate, still-open code change to cut that dependency. Could a committer confirm whether #4711 should be closed as superseded?

Automated update via Claude on behalf of the contributor.