Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
101 lines
11 KiB
Markdown
101 lines
11 KiB
Markdown
# multi-provider-openrouter learnings
|
|
|
|
## Task 1: Extend DispatchProvider config model for OpenRouter
|
|
|
|
- `DispatchProvider` in `src/config.py` now carries `has_energy_telemetry: bool = False` and `enabled: bool = True`.
|
|
- Config is strict (`extra="forbid"`) — adding new keys to `dispatch_providers` entries without matching model fields would fail at load.
|
|
- `config/config.yaml` keeps `neuralwatt` enabled with energy telemetry on, and adds an `openrouter` provider entry without telemetry.
|
|
- `.env.example` now lists both `NEURALWATT_API_KEY=` and `OPENROUTER_API_KEY=`.
|
|
- Full suite passes (1251 tests). Verification script exits 0.
|
|
|
|
|
|
## Task 5: Multi-provider main loop in poller
|
|
|
|
- Refactored `poller.main()` to iterate over `cfg.dispatch_providers.items()` instead of always polling `dispatch_settings.default_provider`.
|
|
- `main()` now skips disabled providers, dispatches `neuralwatt` rows via `fetch_neuralwatt()` and `openrouter` rows via `fetch_openrouter()`, and logs/skips unknown provider keys.
|
|
- Per-provider fetch failures (including `CatalogTooSmall` and zero-row catalogs) are caught and logged; other enabled providers continue unaffected.
|
|
- `mark_stale()` gained an optional `provider=` kwarg that scopes the staleness update with `AND provider = ?`; calling without `provider` retains the historical global behavior, preserving any external callers/tests.
|
|
- After all providers are processed, `main()` calls `tier.apply_tiering(conn, cfg)` so freshly polled rows are immediately routable without a separate `tier` run.
|
|
- Existing freshness tests in `tests/test_poller_freshness.py` were updated to isolate the `neuralwatt` provider under test and to expect provider-skip semantics rather than process abort on fetch error/zero rows.
|
|
- Added `tests/test_multi_provider_poller.py` with tests for: two providers both upserting, failure isolation (one fails, other succeeds), disabled provider skip, zero-row provider skip log, per-provider `mark_stale` scoping, unknown provider key skip, and backward-compatible global `mark_stale`.
|
|
- Verification: `python -m py_compile src/poller.py tests/test_multi_provider_poller.py` passes; targeted pytest suite passes (64 tests).
|
|
|
|
|
|
## Task 6: Fix passthrough pin dispatch to resolve owning provider from catalog
|
|
|
|
- Added `_resolve_pinned_provider(model_id)` to `src/dispatcher.py` that queries `models` for active rows across configured providers, preferring `cfg.dispatch_settings.default_provider` when the same `model_id` exists on multiple providers.
|
|
- Updated alias stripping to handle opencode's `llm-router/<vendor>/<model>` prefix before falling back to `rsplit("/", 1)[-1]`; this preserves full OpenRouter ids like `openai/gpt-6-astra` instead of mangling them to `gpt-6-astra`.
|
|
- Passthrough branch now uses `_resolve_pinned_provider(requested) or cfg.dispatch_settings.default_provider` and passes the resolved provider to `_check_pinned_capabilities(...)`.
|
|
- Added three tests in `tests/test_chat_completions.py` covering OpenRouter-only pin dispatch, `llm-router/openai/gpt-6-astra` prefix handling, and Neuralwatt-only pins still using the default provider.
|
|
- Full suite remains green; changed path verified by `tests/test_chat_completions.py`.
|
|
|
|
|
|
## Task 8: Admin provider CRUD tests
|
|
|
|
- Expanded `tests/test_admin_providers.py` from 3 to 8 tests using isolated temporary config directories.
|
|
- DELETE `/admin/api/providers/neuralwatt` (the current `dispatch_settings.default_provider`) returns 422 and leaves both `config.yaml` and `config.local.yaml` untouched.
|
|
- DELETE `/admin/api/providers/openrouter` (base-configured, non-default provider) returns 403 with a detail naming `config/config.yaml` and read-only provenance.
|
|
- DELETE `/admin/api/providers/nonexistent` returns 404 and does not modify `config.yaml`.
|
|
- POST `/admin/api/providers/testprov` writes only to `config.local.yaml`, leaves `config.yaml` unchanged, and creates a `config.local.yaml.bak.<ts>` backup.
|
|
- GET `/admin/api/providers` returns the merged view, including both base providers (`neuralwatt`, `openrouter`) and an overlay provider (`overlayprov`).
|
|
- Verification: `PYTHONPATH=src python -m pytest tests/test_admin_providers.py -x --tb=short` passes (8 tests, ~2 s). No real provider APIs are called because the tests target the admin router's in-memory config handling only.
|
|
|
|
|
|
- Added `fetch_openrouter(provider)` and `parse_openrouter_model(raw_model)` to `src/poller.py`.
|
|
- Virtual routers (`openrouter/auto`, `openrouter/auto-beta`, `openrouter/free`, `openrouter/fusion`, `openrouter/pareto-code`, `openrouter/bodybuilder`) are dropped before becoming rows.
|
|
- Variant suffixes are preserved; `:batch` maps to `latency_class='flex'`, other variants and base rows to `'standard'`.
|
|
- Catalog `id` is used as `model_id`; `canonical_slug` is used as `base_model_id`.
|
|
- Per-token USD strings from `pricing.prompt` and `pricing.completion` are converted to cost per 1M tokens.
|
|
- `pricing.web_search` and `pricing.cache_read` differentials are intentionally omitted from catalog cost estimates.
|
|
- Added `tests/test_openrouter_poller.py` with 15 mocked tests covering row construction, variant handling, virtual-router exclusion, malformed prices, and request-exception propagation.
|
|
- Full suite passes (1266 tests).
|
|
|
|
|
|
## Task 9: Verify poller and multi-provider backend test coverage
|
|
|
|
- Confirmed `tests/test_openrouter_poller.py` already covers: `fetch_openrouter()` row construction, virtual-router exclusion, `:batch` -> `latency_class='flex'`, `:free`/base -> `'standard'`, malformed pricing handling, and request exception propagation.
|
|
- Confirmed `tests/test_multi_provider_poller.py` already covers: two mocked providers both upsert, failure isolation, disabled provider skip, zero-row provider skip, per-provider `mark_stale()` scoping, unknown provider key skip, and backward-compatible global `mark_stale()`.
|
|
- Added `test_in_process_tiering_runs_after_upsert` to explicitly verify that `main()` calls in-process tiering so OpenRouter rows receive a non-null `tier` immediately after the poll.
|
|
- Targeted verification passes: `PYTHONPATH=src python -m pytest tests/test_openrouter_poller.py tests/test_multi_provider_poller.py tests/test_poller_freshness.py -x --tb=short` -> 30 passed.
|
|
- poller.py coverage on these files: 199 stmts, 12 miss, 94%.
|
|
- Evidence recorded in `.omo/evidence/task-9-multi-provider-openrouter.json`.
|
|
|
|
|
|
## Task 7: Per-provider telemetry and refusal isolation
|
|
|
|
- `extract_telemetry({})` now safely returns a `Telemetry()` instance with all `None` fields; added an explicit non-dict guard so a malformed payload does not crash downstream logging.
|
|
- `_sniff_telemetry_line(": OPENROUTER PROCESSING")` returns `None` because only comments whose first word is exactly `"energy"` or `"cost"` are treated as telemetry.
|
|
- Replaced module-level `_last_account_refusal: float` with `_provider_refusal_since: dict[str, float]`.
|
|
- Renamed `_record_account_refusal()` to `_record_provider_refusal(provider)` and keyed the timestamp by provider.
|
|
- Updated the classifier cascade refusal check (around line 654) to consult `_provider_refusal_since.get(cfg.dispatch_settings.default_provider)` using `cfg.classifier.cooldown_seconds`.
|
|
- Updated non-streaming and streaming dispatch refusal triggers to call `_record_provider_refusal(provider)`.
|
|
- SSE `: energy` / `: cost` sniffing is now gated by `cfg.dispatch_providers[provider].has_energy_telemetry`; when false, telemetry comments are dropped rather than proxied.
|
|
- Added a module `_self_check()` that runs at import time to lock `extract_telemetry({})` and `_sniff_telemetry_line` behavior.
|
|
- Targeted tests `tests/test_classifier_cascade.py` and `tests/test_local_dispatch_fallback.py` fail as expected because they still reference the removed `_last_account_refusal` / `_record_account_refusal` symbols; task 10 will migrate those fixtures.
|
|
- Verification: `python -m py_compile src/dispatcher.py` passes; `PYTHONPATH=src python -c "from dispatcher import extract_telemetry; t = extract_telemetry({}); assert t.energy_kwh is None"` passes.
|
|
- Evidence recorded in `.omo/evidence/task-7-multi-provider-openrouter.json`.
|
|
|
|
|
|
## Task 10: migrate classifier cascade tests to per-provider refusal symbols
|
|
|
|
- Updated `tests/test_classifier_cascade.py` `_clean_state` fixture: replaced the removed `_last_account_refusal` monkeypatch with `dispatcher._provider_refusal_since.clear()` (reset both before and after each test).
|
|
- Renamed `test_account_refusal_skips_the_cloud_step` to `test_provider_refusal_skips_the_cloud_step` and replaced `_record_account_refusal()` with `_record_provider_refusal("neuralwatt")`.
|
|
- Verified `tests/test_local_dispatch_fallback.py` contains no references to `_last_account_refusal` / `_record_account_refusal`; the existing tests continue to pass because `_provider_refusal_since` is scoped per provider and the helper unit tests exercise `_account_level_refusal()` semantics unchanged.
|
|
- Added `tests/test_multi_provider_dispatch.py` with two focused tests:
|
|
- `test_openrouter_streaming_ignores_keepalive_comments_and_logs_null_telemetry`: a streamed OpenRouter response containing `: OPENROUTER PROCESSING` keeps the comment in the proxied body, does not crash, and writes an `energy_observations` row with `energy_kwh` and `cost_usd` NULL.
|
|
- `test_provider_refusal_is_isolated_between_providers`: recording a refusal on `neuralwatt` skips the cloud classifier when `default_provider=neuralwatt`, but does not skip it when `default_provider=openrouter`.
|
|
- Targeted run: `PYTHONPATH=src python -m pytest tests/test_classifier_cascade.py tests/test_local_dispatch_fallback.py tests/test_multi_provider_dispatch.py -x --tb=short` → 39 passed.
|
|
- Full suite: `PYTHONPATH=src python -m pytest --tb=short` → 1288 passed, zero failures.
|
|
- Evidence recorded in `.omo/evidence/task-10-multi-provider-openrouter.json`.
|
|
|
|
|
|
## Task 11: Final verification — Playwright smoke and full test suite
|
|
|
|
- Threw away uvicorn on port 8081 (never touched port 8080 / systemd). Verified `/admin/providers` and `/admin/api/providers` return 200 before driving the UI.
|
|
- Playwright (Chromium, headless) smoke executed at 1400px and 800px.
|
|
- Passed: page renders, Providers navbar link visible, base providers `neuralwatt` and `openrouter` displayed, create provider `smoketest` appears in the list, XSS payload `<script>alert('xss')</script>` is escaped in the DOM, and 30s idle produced zero console errors.
|
|
- **Failed / defect found:** newly-created overlay providers render with source badge `unknown` and no edit/delete buttons because `GET /admin/api/providers` omits `in_base`/`in_overlay` provenance fields. The UI (`providers.html`) keys editability off `in_overlay`, so the toggle and delete flow cannot be exercised via the real page. Root cause: `src/admin.py:admin_providers_list()` returns only `name`, `base_url`, `api_key_env`, `has_energy_telemetry`, `enabled`; it does not include the provenance flags returned for profiles.
|
|
- Full pytest suite passes: 1288 passed, 0 failed. Command: `PYTHONPATH=src python -m pytest --tb=short`.
|
|
- Overlay cleanup: restored `config/config.local.yaml` to its pre-test state (`local_energy` block only); backup files created by `_persist_to` remain in `config/` but are gitignored; `git status` is clean.
|
|
- Evidence, screenshots (1400/800), and console log captured in `.omo/evidence/task-11-multi-provider-openrouter.json`.
|