feat(core,storage): add user-overridable model facts

[Nyvo-io]

Summary

Adds a versioned, schema-validated model-facts.json authority keyed by provider:model. User overrides are projected over provider inventory and generated metadata for catalog, connection-test, and execution paths; enabled override-only models remain selectable without exposing unrelated models. Replacing facts atomically persists validated data and invalidates prior connection verification.

Fixes #2330

Verification

• npm --workspace @maka/core run build
• npm --workspace @maka/storage run build
• npm --workspace @maka/core run typecheck
• npm --workspace @maka/storage run typecheck
• Core full suite: 540 passed
• Focused model-facts suites: 68 passed
• Storage full suite: 781 passed, 14 skipped; one unrelated environment failure because this worktree has no node_modules/dugite/git/bin/git for bundled-git-workspace-smoke.test
• biome check on all 15 changed files
• git diff --check

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the change and tests, and performed local static and test review. The final commit contains Generated-by: Codex.

Checklist

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

Does this PR entail a change in behavior?

• Yes - described under Summary above
• No

[coderabbitai]

📝 Walkthrough

Summary

This PR adds a versioned, schema-validated model-facts.json authority keyed by provider:model.

It solves these problems:

• Users can override context windows, token limits, capabilities, modalities, and pricing.
• Users can select custom models that provider catalogs do not list.
• User fields take precedence over provider and generated facts.
• Effective model facts apply to catalogs, context budgeting, connection tests, and execution.
• Invalid, malformed, oversized, or unsafe documents fail closed.
• Valid replacements persist atomically and invalidate prior connection verification.
• Override-only models become selectable without exposing unrelated models.

Source of truth

This PR extends the existing model-fact sources. It does not replace provider inventories or generated built-in facts.

The effective order is:

1. User overrides.
2. Provider inventory.
3. Generated built-in facts as an offline fallback.

The provider:model key prevents collisions between providers that use the same model ID.

Complexity delta

The PR adds:

• A persisted model-facts document with schema and size limits.
• Validation, normalization, projection, diagnostics, fingerprints, and generation tracking.
• Projection branches for catalogs, connections, execution, onboarding, model fetching, and connection tests.
• Public ./model-facts and runtime-policy store APIs.
• Tests for projection, persistence, recovery, validation, and race handling.

It removes no existing authority. Generated facts remain read-only, and provider inventories remain available.

The added complexity supports custom models, field-level precedence, atomic replacement, restart recovery, unsafe-input rejection, and connection-test invalidation. No safe deletion or simplification is apparent without weakening behavior or regression coverage.

Total maintenance complexity increases, but the increase is justified by the required persisted override behavior. Optional follow-up concerns include surfaced diagnostics, external-edit generation handling, dead catalog-pipeline code, precedence documentation, repeated projections, diagnostic classification, late byte-budget validation, and enabled-ID trimming.

Risks and validation

Effective model facts now affect model visibility, context budgeting, connection tests, execution, and behavior across restarts.

Persisted replacements and external file changes can affect verification invalidation and in-flight test results.

The new ./model-facts export and runtime-policy store methods expand public contracts.

The tests cover provider-specific keys, field merging, capability preservation, custom models, validation, bounded persistence, malformed files, prototype-key rejection, restart recovery, cleanup, rollback, external edits, and pending-ticket supersession.

The repository defines build, typecheck, lint, formatting, and Core/Storage test commands. The supplied context reports these checks as passing, but the shell output provides no direct execution results. Final required-check status therefore remains unverified.

Review-relevant risks

The diff has apparent user-visible behavior and public-contract effects through model selection, catalog projection, connection testing, execution, the ./model-facts export, and runtime-policy store APIs. Material changes in these areas require independent human review under repository policy.

The diff has an apparent persistence and operational-integrity effect through atomic model-facts.json replacement, bounded reads, recovery cleanup, fingerprints, generation tracking, and verification invalidation. Material changes in these areas require independent human review under repository policy.

The diff has an apparent security effect through schema validation and prototype-key rejection. Material changes in this area require independent human review under repository policy.

No licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

This change adds validated model-fact overrides for metadata and capabilities. It persists overrides in model-facts.json, exposes them through runtime-policy stores, applies them to catalogs and execution connections, and invalidates connection tests when facts change.

Changes

Model facts override flow

Layer / File(s)	Summary
Model facts contracts and normalization packages/core/src/model-facts.ts, packages/core/src/runtime-policy/connection-catalog-codec.ts, packages/core/src/__tests__/model-facts.test.ts, packages/core/src/__tests__/runtime-policy-codec.test.ts, packages/core/package.json	Adds model-fact schemas, key construction, strict validation, field merging, modality handling, and the public package export.
Catalog override projection packages/core/src/model-catalog.ts, packages/core/src/__tests__/model-catalog.test.ts	Applies overrides to existing and missing catalog entries, merges capabilities, preserves webSearch, and reports user_override provenance.
Model facts persistence and store facades packages/storage/src/model-facts-store.ts, packages/storage/src/runtime-policy-stores.ts, packages/storage/src/index.ts, packages/storage/src/runtime-policy/document-io.ts, packages/storage/src/__tests__/model-facts-store.test.ts	Adds bounded model-facts reads and writes, validation errors, store facades, public exports, and temporary-file cleanup.
Runtime policy integration and lifecycle validation packages/storage/src/runtime-policy/coordinator.ts, packages/storage/src/__tests__/runtime-policy-model-facts.test.ts, packages/storage/src/__tests__/runtime-policy-stores.test.ts	Projects runtime-policy results through overrides, clears connection-test results after replacements, validates projected ticket state, and verifies restart persistence and protocol selection.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to c5c2f

Pricing overrides are currently rejected, so the advertised user-defined pricing behavior will not work. Merge should wait until the supported pricing fields are accepted and their merge behavior is verified.

Sequence Diagram(s)

sequenceDiagram
  participant RuntimePolicyStoresWriter
  participant RuntimePolicyCoordinator
  participant ModelFactsDocumentOwner
  participant CatalogSnapshot
  RuntimePolicyStoresWriter->>RuntimePolicyCoordinator: replaceModelFacts(overrides)
  RuntimePolicyCoordinator->>ModelFactsDocumentOwner: replace(root, overrides)
  ModelFactsDocumentOwner-->>RuntimePolicyCoordinator: validated ModelFactsDocument
  RuntimePolicyCoordinator->>CatalogSnapshot: apply model-fact overrides
  CatalogSnapshot-->>RuntimePolicyStoresWriter: projected runtime-policy result

Loading

Suggested reviewers: m4n5ter

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: adding user-overridable model facts across core and storage.
Description check	✅ Passed	The description includes the required summary, issue reference, verification results, AI disclosure, checklist, and behavior-change declaration.
Linked Issues check	✅ Passed	The changes implement the linked issue objectives for keyed, validated, field-level model-fact overrides, custom models, precedence, persistence, and catalog integration.
Out of Scope Changes check	✅ Passed	The changed implementation and tests are directly related to model-fact overrides, persistence, runtime integration, validation, and verification invalidation.
Ai Use Disclosure	✅ Passed	The PR selects generative tooling, names Codex and its implementation scope, and all 3 PR commits contain standalone Generated-by: Codex trailers.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 21de45db-4119-444a-a30e-50161c8d0fb3

📥 Commits

Reviewing files that changed from the base of the PR and between 93c65c7 and 3e8b439.

📒 Files selected for processing (15)

• packages/core/package.json
• packages/core/src/__tests__/model-catalog.test.ts
• packages/core/src/__tests__/model-facts.test.ts
• packages/core/src/__tests__/runtime-policy-codec.test.ts
• packages/core/src/model-catalog.ts
• packages/core/src/model-facts.ts
• packages/core/src/runtime-policy/connection-catalog-codec.ts
• packages/storage/src/__tests__/model-facts-store.test.ts
• packages/storage/src/__tests__/runtime-policy-model-facts.test.ts
• packages/storage/src/__tests__/runtime-policy-stores.test.ts
• packages/storage/src/index.ts
• packages/storage/src/model-facts-store.ts
• packages/storage/src/runtime-policy-stores.ts
• packages/storage/src/runtime-policy/coordinator.ts
• packages/storage/src/runtime-policy/document-io.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

[Nyvo-io]

Final revision is ready for maintainer review.

Addressed both CodeRabbit findings in one follow-up commit: prototype-key rejection now fails closed, and model-fact replacement now invalidates verification before persistence while superseding in-flight connection-test tickets.

Verification is complete: all GitHub checks pass, CodeRabbit reports no actionable comments, and the PR is mergeable. Local verification also passed Core 540/540 and the focused model-facts suites; Storage passed 783 tests with 14 platform skips, with the one unrelated bundled-Git smoke failure documented in the PR description.

[Astro-Han]

Thanks for the rework — I verified the architecture is right: read-time projection (overrides never enter the persisted catalog, so session history stays clean — sessions store slug+modelId and re-resolve at execution), user > provider > metadata precedence with field-level capability merge, protected id, schema-validated fail-closed file, atomic replaceModelFacts write, and — critically — the execution path really is wired (resolveExecutionConnection projects, then resolveSelectedModelContextWindow reads the override, so the headline scenario "override contextWindow instead of falling back to 16K" works). The second commit's fail-closed clear-then-write race fix and prototype-pollution defense are sound, and the priority order matches the issue's cross-tool research consensus. CI 14/14 green.

Conclusion: PASS with two P2s that need handling or an explicit deferral note.

P2-1 — the only production entrypoint is hand-editing the JSON file, and a bad file silently disables all overrides with zero diagnostics. readWithDiagnostics returns a diagnostic, but every consumer takes only .document.overrides (coordinator.ts:1441-1443, :639-642, :1041-1046, :1244-1246), and grep confirms nothing in runtime-host/cli/ui references modelFacts/replaceModelFacts — the API has no production callers. A user who typo's model-facts.json (the only live path) loses every override silently, and the "bad models.json → error surfaced, built-ins still work" requirement from #2330 is only half met. At minimum surface the diagnostic (a warn log on the projection read, and an integration test that a bad file through the coordinator disables overrides — today only the store layer tests this).

P2-2 — the invalidation semantics only hold for the (uncalled) API path, not for the file-edit path that is the real entrypoint. modelFactsGeneration is only bumped by replaceModelFacts; an external edit of the file doesn't move it, and sameConnectionTestModelBasis compares only id+apiProtocol — so editing only contextWindow keeps a stale lastTest: verified badge and doesn't cancel an in-flight connection test (whose result then commits under the new facts). Either derive the generation from the file's mtime/stat on the read path, or explicitly document "file edits are advisory; invalidation only happens via replaceModelFacts". Also: replaceModelFacts clearing lastTest for all connections (including unrelated providers) is over-invalidation — consider narrowing to the affected provider/model.

P3 (optional): the modelFactOverrides pipeline in model-catalog.ts is dead in production (no caller passes it; the desktop model menu reads the storage projection) — ~240 lines of code+tests serving only itself; delete if the upcoming UI PR won't consume it; the webSearch capability passthrough is a behavior change for unoverridden provider models too (previously discarded, now surfaced in the catalog) — not declared; dual authority with relayModelProfiles (per-connection, relay-specific): on relay connections the relay profile silently wins over the global file for overlapping fields (contextWindow/vision) — the precedence is undocumented; every catalog snapshot / connection resolution / test-start re-reads and re-freezes the full projection even with no overrides — a stat/mtime cache would help; the error.message.includes('exceeds') diagnostic classification is fragile to wording changes in document-io.ts; 512 overrides × 2048-char strings can exceed the 256KB byte cap after prepareReplacement clears lastTests → commit_outcome_unknown with verification discarded but overrides not written — byte-budget check belongs in prepare; untrimmed enabledModelIds are pushed verbatim as override-entry ids while lookup trims — a whitespace id creates a dirty entry.

---

AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the projection/priority/invalidation paths and grepped for production callers. P2-1 and P2-2 are consequences of the caller graph (both surfaced as unverified predictions). Please weigh these findings with your own judgment.

中文摘要（AI 辅助审查）

结论：PASS（2 个 P2 需处理或显式延后）。架构正确：读时投影（覆盖不进持久化 catalog、Session 历史干净——session 只存 slug+modelId 执行时重解析）、优先级 user>provider>metadata+字段级 capability merge、id 受保护、schema 校验 fail-closed、replaceModelFacts 原子写，执行路径真打通（resolveExecutionConnection 投影后 resolveSelectedModelContextWindow 取覆盖值，headline 场景"覆盖 window 而非 16K 回退"成立）；第二 commit 的 fail-closed clear-then-write 竞态修复与原型污染防御正确；优先级顺序符合 issue 跨工具调研共识。CI 14/14 绿。P2-1：唯一生产入口是手改 JSON，坏文件静默失效全部覆盖零诊断——readWithDiagnostics 返回 diagnostic 但所有消费者只取 .document.overrides，且 grep 确认 runtime-host/cli/ui 零引用 modelFacts/replaceModelFacts（API 无生产调用者）；打错一个字符所有覆盖静默消失，#2330 的"坏文件→error surfaced、内置照常"只完成一半。至少：投影读路径 warn 日志 + "坏文件经 coordinator 投影后覆盖禁用"集成测试（现只测到 store 层）。P2-2：作废语义只对（无人调用的）API 路径成立——modelFactsGeneration 仅 replaceModelFacts 递增，文件外部编辑不改变；sameConnectionTestModelBasis 只比较 id+apiProtocol，仅改 contextWindow 的编辑保留过期 lastTest: verified 且不拦截飞行中 connection test（后者以旧事实测试结果提交到新事实下）。建议：读路径用文件 mtime/stat 派生 generation，或显式声明"文件编辑为 advisory，作废仅经 replaceModelFacts"；另 replaceModelFacts 对所有连接（含无关 provider）清 lastTest 属过度作废，建议按受影响 provider/model 收窄。P3（可选）：model-catalog.ts 的 modelFactOverrides 管线在生产是死代码（无生产调用者，桌面模型菜单走 storage 投影）——若后续 UI PR 不消费应删除（~240 行）；webSearch 能力透传对未覆盖的 provider 模型也是行为变更（此前被丢弃现在出现在 catalog）未声明；与 relayModelProfiles 双权威（relay 连接上 relay profile 静默压过全局文件的重叠字段）优先级未文档化；每次 catalog 快照/resolve/test-start 都重读+refreeze 全量投影（可加 stat/mtime 缓存）；错误分类依赖 error.message.includes('exceeds') 脆；512 覆盖×2048 字符可能超 256KB 字节上限且发生在 prepareReplacement 清 lastTests 之后→commit_outcome_unknown（验证已丢覆盖未写入），字节预算应在 prepare 阶段；未 trim 的 enabledModelIds 原样 push 而 lookup trim——空白 id 产生脏条目。

[Astro-Han]

The field-level precedence (user > provider > metadata), immutable persisted authority, atomic replacement, generation tickets, and execution projection are coherent. Existing public threads already cover malformed-file diagnostics and external-edit invalidation, so I am not duplicating them.

One additional transaction-ordering issue remains below. The first-principles rule is that the complete candidate document—including serialized byte budget—must be validated before any dependent state is invalidated. More broadly, a content fingerprint can be the single authority for both external-edit generation and test invalidation, instead of maintaining parallel API-only generation semantics.

Review performed with three Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I reproduced the finding on the latest head. The branch also needs rebasing because GitHub currently reports conflicts with main.

中文评论

字段级 precedence（user > provider > metadata）、不可变持久权威、原子替换、generation ticket 和执行投影整体闭环。现有公开线程已覆盖 malformed-file diagnostic 与外部编辑失效，我不重复。

以下仍有一个额外的事务顺序问题。第一性原理是：完整候选文档（包括序列化字节预算）必须在任何依赖状态失效之前完成验证。更进一步，可用内容 fingerprint 作为外部编辑 generation 与测试失效的单一权威，避免维护只对 API 写入生效的并行 generation 语义。

本次审查使用了三位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具；我已在最新 head 上复现该问题。当前 GitHub 还显示分支与 main 冲突，需要 rebase。

[Astro-Han]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Uncertain replacement keeps tickets valid ✓ Resolved 🐞 Bug ☼ Reliability

Description

Fix-now: modelFactsGeneration advances only after writeReplacement() returns, but the writer can
publish the renamed facts file and then throw commit_outcome_unknown when directory sync fails. In
that state the new facts are active while an in-flight connection-test ticket retains the old
generation and may commit a fresh verified result, especially for fact changes outside the narrow
API-protocol model basis.

Code

packages/storage/src/runtime-policy/coordinator.ts[R238-240]

+        const persisted = await this.modelFacts.writeReplacement(root, document);
+        this.modelFactsGeneration += 1;
+        return deepFreeze(persisted);

Relevance

●●● Strong

Recent reliability precedents accept fixes for crash windows and stale persisted state after partial
commits.

PR-#1742

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The document writer renames before syncing the directory and explicitly reports an unknown outcome
if a later step fails; the coordinator increments generation only after that call resolves.
Completion relies on generation mismatch, while the fallback semantic model basis contains only
enabled IDs, source, IDs, and API protocol, so other newly published fact changes do not supersede
the old ticket.

packages/storage/src/runtime-policy/coordinator.ts[222-249]
packages/storage/src/runtime-policy/document-io.ts[142-182]
packages/storage/src/runtime-policy/coordinator.ts[1234-1263]
packages/storage/src/runtime-policy/connection-catalog-document.ts[634-660]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A post-rename durability failure can leave new facts published without advancing the generation used to supersede in-flight connection tests.

## Issue Context
Reuse the existing generation invalidation seam: when replacement reports an unknown commit outcome, conservatively advance the generation before propagating the error. Add a fault-injection test covering failure after rename/publication.

## Fix Focus Areas
- packages/storage/src/runtime-policy/coordinator.ts[237-248]
- packages/storage/src/runtime-policy/document-io.ts[155-182]
- packages/storage/src/__tests__/runtime-policy-model-facts.test.ts[86-112]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Sparse modalities bypass validation ✓ Resolved 🐞 Bug ≡ Correctness

Description

Fix-now: decodeModelModalities() uses Array.map, which skips holes, so a programmatic input such
as {input: Array(1), output: ['text']} is accepted with an unvalidated sparse modality. The
malformed model can then pass normalizeConnectionModelDiscoveryResult() and enter the
catalog/runtime contract.

Code

packages/core/src/runtime-policy/connection-catalog-codec.ts[R518-519]

+  const input = item.input.map((entry) => decodeModelInputModality(entry));
+  const output = item.output.map((entry) => decodeModelOutputModality(entry));

Relevance

●●● Strong

Exact recent precedent accepted rejecting sparse arrays caused by map holes.

PR-#3079

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new modality decoder maps arrays directly, while the discovery normalizer trusts each decoded
model; JavaScript map preserves and skips sparse slots. The same root-cause pattern was accepted
in PR #3079.

packages/core/src/runtime-policy/connection-catalog-codec.ts[513-520]
packages/core/src/runtime-policy/connection-catalog-codec.ts[566-592]
PR-#3079

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Sparse modality arrays bypass decoding because `Array.map()` skips holes.

## Issue Context
This is the same decoder pattern previously fixed for sparse protocol arrays; use an iteration form that visits every numeric slot so holes are decoded as invalid values.

## Fix Focus Areas
- packages/core/src/runtime-policy/connection-catalog-codec.ts[513-520]
- packages/core/src/__tests__/runtime-policy-codec.test.ts[376-401]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Oversized input misclassified ✓ Resolved 🐞 Bug ≡ Correctness

Description

Non-blocking remediation: an otherwise schema-valid replacement can exceed 256 KiB because up to 512
overrides may each contain 2,048-character fields, but the size is checked only by the persistence
writer and therefore surfaces as invalid_document instead of caller-facing invalid_policy_input.
Runtime-policy consumers classify that as a persistence failure rather than an invalid request.

Code

packages/storage/src/model-facts-store.ts[R77-79]

+  async writeReplacement(root: string, document: ModelFactsDocument): Promise<ModelFactsDocument> {
+    await writeJsonDocument(root, FILE, document, MODEL_FACTS_DOCUMENT_MAX_BYTES);
+    return document;

Relevance

●● Moderate

Clear classification issue, but no close precedent confirms preflight byte-size validation for this
new store.

PR-#3028

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Preparation bounds shape and count but not encoded bytes; writeJsonDocument applies the byte limit
through invalidDocument, while the established policy mutation path checks serialized size during
preparation and raises invalid_policy_input. The runtime host maps those two codes to different
public failures.

packages/storage/src/model-facts-store.ts[49-79]
packages/core/src/model-facts.ts[52-60]
packages/core/src/model-facts.ts[82-87]
packages/storage/src/runtime-policy/document-io.ts[133-140]
packages/storage/src/runtime-policy/policy-document.ts[85-96]
packages/runtime-host/src/server/runtime-policy-coordinator.ts[320-339]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Oversized replacement input is classified as a corrupt/persistence document instead of invalid policy input.

## Issue Context
Reuse `serializeJsonDocument()` during `prepareReplacement()`, as the existing runtime-policy document owner does; no new authority or public surface is needed.

## Fix Focus Areas
- packages/storage/src/model-facts-store.ts[49-79]
- packages/storage/src/__tests__/model-facts-store.test.ts[10-30]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Web pages:

  🌐 test

  🌐 test(1) - Linux manual page

  🌐 Getting Started · Jest

  +2 more

Review mode: 🧠 Deep: This is a bug-dense behavioral change spanning core projection, schema validation, storage persistence, public store APIs, catalog/connection-test paths, and verification invalidation, with many independent logic sites where redundant review can catch subtle defects.

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ba670128-8656-4efd-a2c1-b5feb82a2d33

📥 Commits

Reviewing files that changed from the base of the PR and between 0ef1c55 and c5c2fa1.

📒 Files selected for processing (15)

• packages/core/package.json
• packages/core/src/__tests__/model-catalog.test.ts
• packages/core/src/__tests__/model-facts.test.ts
• packages/core/src/__tests__/runtime-policy-codec.test.ts
• packages/core/src/model-catalog.ts
• packages/core/src/model-facts.ts
• packages/core/src/runtime-policy/connection-catalog-codec.ts
• packages/storage/src/__tests__/model-facts-store.test.ts
• packages/storage/src/__tests__/runtime-policy-model-facts.test.ts
• packages/storage/src/__tests__/runtime-policy-stores.test.ts
• packages/storage/src/index.ts
• packages/storage/src/model-facts-store.ts
• packages/storage/src/runtime-policy-stores.ts
• packages/storage/src/runtime-policy/coordinator.ts
• packages/storage/src/runtime-policy/document-io.ts

🚧 Files skipped from review as they are similar to previous changes (12)

• packages/storage/src/index.ts
• packages/storage/src/tests/runtime-policy-stores.test.ts
• packages/core/src/tests/runtime-policy-codec.test.ts
• packages/core/package.json
• packages/core/src/tests/model-facts.test.ts
• packages/storage/src/model-facts-store.ts
• packages/storage/src/runtime-policy-stores.ts
• packages/core/src/tests/model-catalog.test.ts
• packages/core/src/runtime-policy/connection-catalog-codec.ts
• packages/core/src/model-catalog.ts
• packages/storage/src/runtime-policy/coordinator.ts
• packages/storage/src/tests/model-facts-store.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.

[Astro-Han]

The immutable facts document, field-level projection, generation tickets, and execution resolver are coherent on this head; I also verified that the execution resolver applies overrides before returning, so an initially suspected execution bypass was discarded. One normal refresh path still removes the headline override-only model state.

The inline P2 is the smallest remaining product correction: model fetch reconciliation needs to preserve currently enabled IDs backed by model facts without writing them into provider inventory. The current live typecheck is also failing, so this head is not merge-ready independently of the review finding.

AI-assisted review by Codex with three independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high) as an advisory pass; I verified the current head, refresh reconciliation, projection semantics, existing threads, and live CI.

中文

当前 head 的不可变 facts document、字段级 projection、generation ticket 与 execution resolver 整体闭环；我也确认 execution resolver 返回前确实应用 overrides，因此剔除了一个初步误报。仍有一个正常刷新路径会删除核心的 override-only model 状态。

行内 P2 是最小剩余修复：model fetch reconciliation 应保留当前已启用且由 model facts 支撑的 ID，但不要把它写进 provider inventory。当前实时 typecheck 也失败，因此即使不考虑 finding，这个 head 也尚不可合并。

本次由 Codex、三个独立 reviewer 与 OpenCode Go DeepSeek V4 Flash high 辅助；已核对当前 head、刷新 reconciliation、projection semantics、已有线程和实时 CI。

[Astro-Han]

The current head is semantically equivalent to the previously reviewed implementation plus rebase/formatting. I verified that the oversized-input validation, sparse-modality validation, and post-publication generation invalidation are now covered; I also resolved the pricing thread because #2330 explicitly keeps pricing in the existing pricing editor rather than duplicating it in model-facts.json.

The existing provider-refresh P2 remains current and unresolved: refresh reconciliation still removes an enabled/default override-only model before the facts projection runs. No duplicate inline is needed. This also changes user-visible model selection by making override-only models selectable, but the PR has no visual evidence; please add an actual TUI or Desktop model-selector screenshot showing a facts-backed custom model. Exact-head CI has not run yet because the Apache Actions lanes are unavailable/queued.

AI-assisted review by OpenAI Codex. I verified the exact-head range-diff, current implementation and tests, issue #2330’s ownership decision, review-thread state, screenshot gate, provenance, and live checks.

中文

当前 head 与上一轮相比主要是 rebase/format；oversized input、稀疏 modalities 和发布后 generation 失效均已有修复。pricing 线程不成立，因为 #2330 明确要求 pricing 继续由现有 pricing editor 负责，不应重复进入 model-facts.json。

已有的 provider refresh P2 仍然有效：刷新会在 facts projection 前删除已启用/默认的 override-only model，因此不重复发 inline。该改动会让自定义模型出现在用户选择器中，也需要补一张真实 TUI 或 Desktop 模型选择器截图。当前 exact-head CI 尚未实际运行。

本次由 OpenAI Codex 辅助，已核对 exact-head range-diff、实现与测试、#2330 权威边界、线程状态、截图门禁、来源披露和实时检查。

[Nyvo-io]

Implemented and reviewed the provider-refresh correction in one follow-up commit: f321119.\n\n- Refresh reconciliation now preserves only facts-backed model IDs that are already enabled or are the connection's current default.\n- Raw provider inventories remain limited to live discovery results; custom facts are projected read-time.\n- The default target remains valid and the execution resolver still resolves the selected custom model.\n- Added Core and Storage regressions covering refresh, default retention, execution, inventory purity, and exclusion of unselected facts-backed models.\n- Rebased onto current main.\n\nVerification on the rebased head:\n- Core typecheck passed.\n- Storage typecheck passed after rebuilding Core.\n- Biome check passed for all changed files.\n- git diff --check passed.\n- Core reconciliation tests: 9 passed.\n- Storage model-facts tests: 6 passed.\n- Storage runtime-policy-stores tests: 44 passed.\n- Independent adversarial review found no actionable defect.\n\nPricing remains intentionally outside model-facts.json, per issue #2330's authority decision; the existing pricing editor remains the pricing source of truth.

[Astro-Han]

Thanks — the storage half of this is genuinely careful, and I want to name that before the findings. Reviewed exact head f321119a7f2ebf3576e1a9bc9decfa2ea8326249.

The store gets the hard mechanics right: atomicity and temp-file recovery, commit_outcome_unknown on both the pre-clear and post-publication failure paths, oversize rejection that preserves verification, external-edit ticket supersession, and __proto__ rejection — with the key grammar handling colon-bearing model ids like ollama-cloud:gpt-oss:120b correctly, because only the provider segment is colon-free. The numeric validator is tighter than the codec's and rejects zero, negatives, non-integers and infinities. Most importantly, no write path reads a projected snapshot and writes it back, so overrides never contaminate connection-catalog.json — that is what keeps a stale pin recoverable, and there is an explicit assertion for it. Pricing is not reachable from this PR at all, since the override allowlist excludes price fields and pricing has its own authority; the wrong-number risk here is truncation, not cost.

The problem is the precedence chain. Context window now has three authorities — relayModelProfiles, this new fact table, and bundled metadata — and resolveSelectedModelContextWindow consults the first one first and returns early, so on an openai-compatible connection a model fact silently loses to a setting the user may have configured months earlier, with no diagnostic. RelayModelProfile is the pre-existing per-model user-declaration seam with a documented gate; the project rule is to extend the closest existing seam rather than run a parallel one beside it, and that is what happened here. Separately, the PR threads a second, entirely dead projection path through the catalog builders, which is why the one provenance signal it adds can never fire.

Two P1s, six P2s and three P3s inline. Not approving while P1/P2 findings are open.

On tests: the storage suite is strong on mechanics and empty on precedence. Nothing asserts a fact override against a competing authority, nothing drives capabilitySource through the call shape production actually uses — which is precisely why the dead path went unnoticed — nothing covers a contextWindow override against a row carrying the inputLimit this same PR starts persisting, and nothing covers a non-1 schemaVersion, a concurrent replace, or an override for a model that no longer exists. Each of those inherits its severity from the finding it belongs to rather than standing on its own.

Review disclosure: this review was prepared with Claude Code, which read the diff at this head, enumerated the reader sites and the precedence chain by search rather than by assumption, and executed several of the pure functions against verbatim copies to check the merge semantics. Evidence grade is stated per finding — three were reproduced by execution, the rest are code reading or labelled inference. The human contributor reviewed this before posting.

[Astro-Han]

Hi — this PR conflicts with current main and cannot be merged as-is.

I tested a rebase onto current main locally (in a throwaway worktree — your branch was not touched). It stops on these files:

• packages/core/src/model-catalog.ts

These are real source conflicts, so they need your judgement rather than a mechanical rebase — please rebase onto current main and resolve them yourself, then push. Once the branch is conflict-free and CI is green on the new head, I will pick it up for review.

git fetch upstream && git rebase upstream/main
# resolve, then
git push --force-with-lease

Thanks for the contribution — happy to help if any conflict is unclear.

---

AI-assisted maintenance note, not a review. It does not count as the required human review under CONTRIBUTING.md §Review.

[Astro-Han]

Reviewed on exact head a70fbf3d3e5395b921ff921f3d8fcb3fa3af2340. No P0–P2. One [P3] inline. Not approving only because test is red — see the end.

Two questions decided whether this feature is safe, and both come out well.

Precedence between user overrides and built-in metadata is coherent, and the same rule is implemented in both layers for the same stated reason. When contextWindow is overridden but inputLimit is not, inputLimit is carried to the new value, with the reasoning written down: a stale, narrower provider inputLimit must not silently win at resolution time. The execution layer agrees — context-budget-policy.ts:441 makes a facts pin on contextWindow take precedence over the older relay-only declaration and then takes narrowestPositiveLimit(contextWindow, inputLimit), justified as "catalog display and actual execution must use the same window." One rule, two layers, one rationale. That is the part I most want to credit.

A bad override cannot brick a model. The write path is strict: normalizeModelFactOverride whitelists fields and throws Unknown model fact field on anything else, caps strings at 2048, requires isPositiveBoundedInteger for numerics, restricts apiProtocol to three enum values, requires keys to match provider:model within 512 characters, and caps the override count at 512. Bad values are rejected as RuntimePolicyStoreError('invalid_policy_input') before they can be persisted. The read path is fail-open: model-facts-store.ts:52-57 returns an empty document plus a diagnostic rather than throwing, so the model falls back to provider facts and stays usable.

Candidates considered and dropped — listed so they can be re-checked rather than re-derived:

• "One typo permanently destroys 511 other overrides." I initially graded this a P2 data-loss issue and withdrew it: the fingerprint is computed from the real bytes before decoding (:48), so optimistic concurrency checks the true file identity. unsupported_schema is explicitly protected from being overwritten while malformed is not, and that asymmetry is principled — unparseable content cannot be merged into, so replacing it is reasonable, whereas a file written by a newer version that this build simply does not understand must never be clobbered. Correct as designed.
• contextWindow has no lower bound (isPositiveBoundedInteger accepts 1). Not reported. This is an explicit user fact override; second-guessing it would defeat the premise of the feature.

Gating: audit is green, test is completed/failure on missing ASF license headers. That is a mechanical fix — running npm run write:asf-headers and pushing should clear it. Nothing on the code side blocks from my end; happy to re-check the terminal state afterwards.

[Nyvo-io]

Rebased onto current main and updated the single commit. The Runtime Host catalog projection now keeps the persisted model-facts verification fingerprint out of the client-visible protocol header; empty model-fact overrides are also rejected before projection. Final CI is green: test and audit passed. Local lint, format, ASF-header, build, typecheck, Core, and focused Runtime Host regression checks also passed.

[Nyvo-io]

Rebased onto current main and pushed the single verified commit 9dc0d5dc4. The exact-head GitHub checks are all green (test, audit, and windows_recovery), and local build, lint, format, ASF-header, Core, Runtime, Storage, and Runtime Host verification passed. No new code changes were made after review. Ready for maintainer review.

[Astro-Han]

Thanks for your patience here. I re-reviewed exact head e28b8217c3; CI is green and I found no remaining P0/P1 issue. The earlier findings around refresh loss, read-side durable writes, context/input-limit precedence, relay precedence, schema downgrade, and provenance are fixed or superseded on this head.

Two non-blocking P2 observations remain:

• display-only fact changes currently participate in the connection-verification fingerprint and can unnecessarily clear a valid verification result;
• a provider inventory at the supported 2,048-entry boundary plus one enabled override-only model can produce catalog index 2,048, beyond the protocol’s accepted 0…2047 range.

I also found several concrete simplifications that should preferably be made in this PR:

• delete the entire factBackedModelIds plumbing: it is passed through several layers but never read by reconcileConnectionAfterModelFetch, whose existing behavior already preserves enabled IDs;
• delete the catalog builder’s raw modelFactOverrides input and its tests: no production caller uses it, while the real Runtime Host path already supplies projected ModelInfo with factOverriddenFields;
• remove the unused ModelFactsDocumentOwner wrappers and fingerprint helper;
• avoid production injection seams that exist only to make one directory-sync test convenient;
• unless a real Runtime Host/IPC writer is included here, remove the currently unconsumed RuntimePolicyStores.modelFacts get/replace facade and introduce it with its first production caller;
• move the two .maka-shots screenshots into the PR evidence and remove the unreferenced binaries from the repository.

These deletions remove parallel or unused paths without changing the user-facing model-facts behavior. With them treated as non-blocking cleanup, I’m approving the current solution.

OpenAI Codex and delegated @reviewer agents assisted with authority, production-path, test, and simplification analysis. Astro-Han reviewed the evidence and made the final approval decision.

中文对照

谢谢你耐心等待。我重新检查了当前 head e28b8217c3；CI 全绿，当前没有剩余 P0/P1。之前关于刷新丢失、读取路径写入、context/input limit 优先级、relay 优先级、schema 降级和 provenance 的意见，在当前 head 上已经修复或失效。

目前只剩两个非阻塞 P2：展示类事实也会参与连接验证 fingerprint，可能不必要地清除有效验证；以及 provider inventory 恰好达到支持的 2,048 项后再加入一个已启用的 override-only model，会生成协议不接受的 index 2,048。

另外有几组可以明确删除的代码：完全没有被 reconciler 消费的 factBackedModelIds 管线、只有测试使用的第二条 raw override projection、未使用的 store wrapper/fingerprint、只服务测试的生产注入 seam，以及当前没有 Host/IPC 消费者的 model-facts writer facade。两张没有在 PR 正文引用的截图也应转为 PR 证据并从仓库删除。

这些删除不会改变用户可见的 model-facts 行为，只会移除平行路径和未使用接口。把它们视为非阻塞清理后，我批准当前方案。