-
Notifications
You must be signed in to change notification settings - Fork 751
fix(routing): source capability evidence from explicit catalog provenance #1799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
e02dd67
docs(devlog): plan the local-model capability evidence and plugin rou…
lidge-jun 25d58e9
docs(devlog): fold audit round 1 blockers into the capability provena…
lidge-jun ec88666
docs(devlog): fold audit round 2 precision fixes into the phase docs
lidge-jun ce116b9
docs(devlog): make the remote exact-head verifier self-contained
lidge-jun 8c74344
docs(devlog): fold audit round 4 blockers and link issues #1796/#1797
lidge-jun 94ec4ce
docs(devlog): fold audit round 5 blockers (fail-closed remote steps, …
lidge-jun 21320bc
docs(devlog): make each remote verifier step self-contained; widen ph…
lidge-jun 99b38c1
docs(devlog): record the six-round audit ledger and verdict-capture c…
lidge-jun 57e4dd7
docs(devlog): capture jawcode generated metadata in provenance (round 7)
lidge-jun adfd40f
docs(devlog): own the lookup extraction, add the context-cap regression
lidge-jun af6bbfc
docs(devlog): record rounds 7-8 and the writer-vs-producer lesson
lidge-jun c435340
fix(routing): source capability evidence from explicit catalog proven…
lidge-jun cc3512e
fix(catalog): read llama.cpp served context from meta.n_ctx (#1797)
lidge-jun f9ef0a3
docs(devlog): record the implementation, verification evidence, and C…
lidge-jun 65f269f
docs: address CodeRabbit review findings on #1799
lidge-jun d3f63e6
docs: address the second CodeRabbit round on #1799
lidge-jun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
devlog/_plan/260816_local_model_capability_and_plugin_routing/000_plan.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # 260816 — Local-model capability evidence and browser-plugin routing | ||
|
|
||
| ## Objective | ||
|
|
||
| A local Qwen model (`lidge/qwen3.8-27b-nvfp4`, llama.cpp behind an | ||
| `openai-chat` adapter) could not drive the Chrome or Computer Use browser | ||
| plugins. Investigating that failure surfaced a defect in | ||
| `src/routing/capability.ts` that is **not local-model specific**: the cached | ||
| Codex catalog is read on every policy-routed request and then discarded in | ||
| full, because the reader expects a field shape the catalog writer never | ||
| produces. | ||
|
|
||
| This unit fixes the capability-evidence defect, records the live-catalog | ||
| ingestion gap, and writes durable routing guidance for weaker local models | ||
| that must reach the browser plugins through the privileged Node REPL tool. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - `src/routing/capability.ts` is on the request path for every policy-routed | ||
| request. The memoized catalog read exists precisely so the parse cost is not | ||
| paid per candidate; a fix must not turn it into a per-candidate parse. | ||
| - "Unknown is not zero" is the module's stated contract (file header): a | ||
| dimension without canonical evidence must stay `undefined`, never `false`. | ||
| The fix must not convert a missing catalog field into a negative assertion. | ||
| - `src/routing/capability.ts` is reachable from `src/router.ts`, so the | ||
| core/lab boundary in `tests/core-lab-boundary.test.ts` applies: no import | ||
| may reach `src/lab/`. | ||
| - Out of scope: promotion to `main`, npm publish, GUI redesign, unrelated | ||
| provider adapters. | ||
|
|
||
| ## Dependency-ordered work-phase map | ||
|
|
||
| The order is build-order, not effort order: the catalog reader is the | ||
| foundation both later phases depend on. | ||
|
|
||
| | Phase | Doc | Depends on | Independently verifiable by | | ||
| |-------|-----|------------|------------------------------| | ||
| | 1 | `010_catalog_row_shape.md` | — | New focused test: catalog-sourced evidence survives for a routed row | | ||
| | 2 | `020_live_capability_ingestion.md` | Phase 1 | Focused test driving the observed llama.cpp `/v1/models` payload | | ||
| | 3 | `030_local_model_plugin_routing.md` | — (docs surface) | The written guidance resolves on the documented path | | ||
|
|
||
| Phase 3 has no code dependency on 1 or 2 and could land in any order; it is | ||
| listed last because it is documentation, not because it is smaller. | ||
|
|
||
| ## Source-of-truth sync target (SOT-SYNC-01) | ||
|
|
||
| `structure/` holds maintainer invariants. Phase 1 changes how routing evidence | ||
| is sourced, so C patches the structure note that describes routing evidence if | ||
| one exists; if none does, the D summary recommends creating it. | ||
|
|
||
| ## Research documents | ||
|
|
||
| - `001_capability_evidence_defect.md` — the reproduction, the field-shape | ||
| mismatch, and why every existing test passes over it. | ||
| - `002_local_model_plugin_failure.md` — why the local model could not reach | ||
| the Chrome and Computer Use plugins. | ||
|
|
||
| ## Filed issues | ||
|
|
||
| | Issue | Covers | Fixed by | | ||
| |-------|--------|----------| | ||
| | [#1796](https://github.com/lidge-jun/opencodex/issues/1796) | Routing discards every catalog row (field-shape mismatch) | Phase 1 | | ||
| | [#1797](https://github.com/lidge-jun/opencodex/issues/1797) | llama.cpp `multimodal` token + dual-envelope join | Deferred; Phase 2 ships the context half only | | ||
|
|
||
| Phase 3 files no issue: it is host-side guidance with no opencodex defect | ||
| behind it (see `002_local_model_plugin_failure.md`). |
140 changes: 140 additions & 0 deletions
140
...816_local_model_capability_and_plugin_routing/001_capability_evidence_defect.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| # 001 — Catalog capability evidence never reaches routing | ||
|
|
||
| Research document. No diffs here; the fix design is `010_catalog_row_shape.md`. | ||
|
|
||
| ## Symptom that started this | ||
|
|
||
| `lidge/qwen3.8-27b-nvfp4` was registered through the supported CLI: | ||
|
|
||
| ``` | ||
| $ ocx models add lidge qwen3.8-27b-nvfp4 --context-window 262144 --modalities text,image | ||
| Error: custom model "lidge/qwen3.8-27b-nvfp4" already exists | ||
| ``` | ||
|
|
||
| The row was already present and complete in `~/.opencodex/config.json`: | ||
|
|
||
| ```json | ||
| { "id": "83ca0b4c-06bb-475d-b585-6c47b9d6be71", "provider": "lidge", | ||
| "modelId": "qwen3.8-27b-nvfp4", "displayName": "Qwen3.8 27B NVFP4 (lidge 5090)", | ||
| "contextWindow": 262144, "inputModalities": ["text", "image"] } | ||
| ``` | ||
|
|
||
| It also reached the on-disk Codex catalog correctly, as | ||
| `/Users/jun/.codex/opencodex-catalog.json`: | ||
|
|
||
| ```json | ||
| { "slug": "lidge/qwen3.8-27b-nvfp4", "context_window": 262144, | ||
| "input_modalities": ["text", "image"], "supports_parallel_tool_calls": true } | ||
| ``` | ||
|
|
||
| Yet routing evidence carried neither value. NOTE (audit round 1, B9): this | ||
| one-liner reproduces only with the provider's `modelContextWindows` and | ||
| `modelInputModalities` ABSENT. They were added by hand later while | ||
| diagnosing, so on today's live config the earlier branches win and the | ||
| symptom is masked. The 17-to-0 catalog proof below is independent of that | ||
| and still reproduces exactly. Use a catalog-only fixture to reproduce: | ||
|
|
||
| ``` | ||
| candidateCapabilityEvidence(config, "lidge", "qwen3.8-27b-nvfp4") | ||
| => { tools: true, serviceTier: "unsupported", encryptedCodexTasks: false } | ||
| ``` | ||
|
|
||
| No `image`. No `contextWindow`. A model registered through the documented | ||
| path is image-blind to the router. | ||
|
|
||
| ## Root cause: reader and writer disagree on field shape | ||
|
|
||
| `src/routing/capability.ts` `cachedCatalogModels()` filters rows with: | ||
|
|
||
| ```ts | ||
| typeof model.id === "string" && typeof model.provider === "string" | ||
| ``` | ||
|
|
||
| and then reads `model.contextWindow` / `model.inputModalities`. | ||
|
|
||
| The catalog file has none of those four fields. Its actual keys are | ||
| `slug` (a combined `provider/id`), `context_window`, and `input_modalities`. | ||
| Verified against the live file: | ||
|
|
||
| ``` | ||
| KEYS: slug,display_name,description,default_reasoning_level, | ||
| supported_reasoning_levels,...,input_modalities,...,context_window, | ||
| max_context_window,auto_compact_token_limit,... | ||
| provider? undefined id? undefined context_window? 262144 | ||
| ``` | ||
|
|
||
| So the filter rejects every row: | ||
|
|
||
| ``` | ||
| TOTAL: 17 | SURVIVING capability.ts filter: 0 | ||
| ``` | ||
|
|
||
| **This is not a local-model bug.** All 17 rows are discarded — native OpenAI | ||
| rows, `anthropic/claude-opus-5`, `xai/grok-4.6`, everything. The whole | ||
| `catalogRow` branch of the evidence chain is dead code in practice. | ||
|
|
||
| ## Why nobody noticed | ||
|
|
||
| The catalog is the *fourth* fallback. For a provider with populated config | ||
| maps the earlier branches answer first: | ||
|
|
||
| ``` | ||
| provider.modelContextWindows[id] ?? provider.contextWindow | ||
| ?? registryEntry.modelContextWindows[id] ?? catalogRow.contextWindow ?? ... | ||
| ``` | ||
|
|
||
| `kimi`, `anthropic`, `xai`, and `alibaba-token-plan-intl` all declare | ||
| `modelContextWindows` and `modelInputModalities` inline, so their evidence | ||
| looks correct and the dead branch stays invisible. Only a provider that | ||
| relies on the catalog — exactly what `ocx models add` produces — is exposed. | ||
|
|
||
| Confirmed by contrast on the same tree: | ||
|
|
||
| ``` | ||
| xai/grok-4.6 => contextWindow 500000, image true (provider maps) | ||
| anthropic/claude-opus-5 => contextWindow 1000000, no image (provider maps) | ||
| lidge/qwen3.8-27b-nvfp4 => nothing (catalog only) | ||
| ``` | ||
|
|
||
| `claude-opus-5` is itself a smaller instance of the same hole: the provider | ||
| block declares `modelContextWindows` but no `modelInputModalities`, and the | ||
| catalog that could have supplied `image` is discarded. | ||
|
|
||
| ## Why the test suite is green | ||
|
|
||
| `tests/routing-profile.test.ts` and `tests/routing-compatibility.test.ts` | ||
| construct `capability` objects inline: | ||
|
|
||
| ```ts | ||
| { provider: "a", model: "m1", capability: { contextWindow: 200000, tools: true } } | ||
| ``` | ||
|
|
||
| They exercise the *policy evaluator* with pre-made evidence and never call | ||
| `candidateCapabilityEvidence`, so no test ever reads a real catalog file. | ||
| The assembly step between the catalog on disk and the evaluator is untested. | ||
| That is the coverage gap this unit closes, and it is why "all tests green" | ||
| said nothing about this defect. | ||
|
|
||
| ## Field-chain note (PLAN-FIELD-CHAIN-01) | ||
|
|
||
| The chain for a custom model is: | ||
|
|
||
| | Stage | Path | State | | ||
| |-------|------|-------| | ||
| | creation | `src/cli/models.ts` `ocx models add` | works | | ||
| | serialization | `config.customModels[]` -> `src/codex/catalog/provider-fetch.ts:1758` | works | | ||
| | on-disk form | `~/.codex/opencodex-catalog.json` (`slug`, `context_window`, `input_modalities`) | works | | ||
| | consumer | `src/routing/capability.ts` `cachedCatalogModels()` | **broken — expects `provider`/`id`/`contextWindow`/`inputModalities`** | | ||
|
|
||
| Only the last stage is wrong, which is why the value is visible everywhere a | ||
| human looks (config, CLI, catalog file) and absent exactly where routing | ||
| decides. | ||
|
|
||
| ## Post-audit correction (round 1) | ||
|
|
||
| The fix originally proposed here — read `context_window` and | ||
| `input_modalities` straight off the row — is WRONG and was rejected in | ||
| audit. `ensureStrictCatalogFields()` synthesizes both fields for Codex's | ||
| strict parser, so reading them would convert unknown into `image:false` | ||
| and a fabricated `128000`. See `003_audit_synthesis_round1.md` B2 and the | ||
| provenance design in `010_catalog_row_shape.md`. |
60 changes: 60 additions & 0 deletions
60
...816_local_model_capability_and_plugin_routing/002_local_model_plugin_failure.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # 002 — Why a local model could not reach the browser plugins | ||
|
|
||
| Research document. Environment observation, not an opencodex code defect; | ||
| the durable-guidance design is `030_local_model_plugin_routing.md`. | ||
|
|
||
| ## What was observed | ||
|
|
||
| A local Qwen model asked to browse with the Chrome and Computer Use plugins | ||
| tried, in order: `node -e "import('@oai/sky')"`, `find` and `mdfind` sweeps | ||
| for a `sky` package, `osascript` against Google Chrome, and finally a | ||
| hand-written `/tmp/chrome_probe.mjs` importing the plugin's | ||
| `browser-client.mjs` directly. It then reported the tooling unavailable. | ||
|
|
||
| The tooling was available the whole time. | ||
|
|
||
| ## Why every shell attempt fails by construction | ||
|
|
||
| `scripts/browser-client.mjs` in the Chrome plugin is a ~1.15 MB bundle that | ||
| expects a privileged host. Importing it from an ordinary Node process | ||
| resolves its exports and then refuses at runtime: | ||
|
|
||
| ```console | ||
| $ node -e "import('.../scripts/browser-client.mjs').then(m => m.setupBrowserRuntime())" | ||
| RUNTIME FAIL: Browser use requires privileged node_repl capabilities | ||
| ``` | ||
|
|
||
| The bundle carries its own `process` shim and reads `globalThis.nodeRepl`; | ||
| those are injected by the privileged REPL host, not by Node. Computer Use is | ||
| stricter still — `@oai/sky` has no on-disk package at all, so filesystem | ||
| searches for it can only ever come back empty. | ||
|
|
||
| The single working entry point is the `mcp__node_repl__js` tool. Driving the | ||
| same plugin through it succeeded immediately in this session: browser bound, | ||
| `chrome.user.openTabs()` returned the live tab list, navigation and a | ||
| screenshot both worked. | ||
|
|
||
| ## Why a weaker model misroutes here | ||
|
|
||
| The Chrome skill deliberately obscures its own mechanism for user-facing | ||
| reasons: | ||
|
|
||
| > "Never mention `Node REPL`, `node_repl`, `REPL`, JavaScript sessions ... | ||
| > unless a user is asking for that exact information." | ||
|
|
||
| while simultaneously requiring it: | ||
|
|
||
| > "Run browser setup code through the Node REPL `js` tool ... If it is not | ||
| > already available, use tool discovery for `node_repl js`." | ||
|
|
||
| A model with strong instruction-following holds both. A weaker one resolves | ||
| the conflict by treating the named tool as off-limits and substitutes a | ||
| shell, which is exactly the observed failure. The skill text plus its | ||
| `documentation()` payload is also ~60 KB before any work begins, which | ||
| compounds the problem for a small local context. | ||
|
|
||
| ## Scope boundary | ||
|
|
||
| This is host/skill routing behavior, not opencodex runtime behavior. Nothing | ||
| in `src/` can fix it. The actionable output is durable guidance on a surface | ||
| a local model actually reads, and that is all Phase 3 does. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.