Private
Public Access
0
0

conductor(track): nagent_review_v3 §5 Provider expansion cluster

This commit is contained in:
2026-06-20 08:17:30 -04:00
parent 62f40d9410
commit dd8428a30f
@@ -250,7 +250,59 @@ The "project memory is team memory" payoff (557dd39's Part IV addition) is the n
## §5 Provider expansion
(filled in by Phase 6 — covers `bdfa2a6`, `5075f6e`, `2edc7ee`)
**Source:** nagent `bdfa2a6`, `5075f6e`, `2edc7ee` (`bin/helpers/nagent_llm.py:13-19` + `:27-31` + `:37-42` + `:54-77` + `:123-130` + `:198-279` + `:315-336` + `:381-400` + `:582-625` + `:739-770` + `:357-391`, `bin/nagent:1075-1081`, `config.example.json:7`, `README.md:82-90` + `:956-967` + `:991-995`, `tests/test_nagent.py:1010-1042` + `:2734-2797`, `context/data-oriented-design.md`).
**One-liner:** Together is added as a sixth provider (OpenAI-wire-compatible, always streamed). Per-model context windows become a verified table; rebuild now fires on whichever trips first — byte ceiling or 0.85 of the model's window. The claude-code provider blanks inherited `ANTHROPIC_API_KEY` so its billing stays on its own login; the spinner names the provider/model.
**Pattern(s) vs v2.3:** UPDATE. v2.3 had 5 providers (openai, anthropic, google, cursor, claude-code); v3 has 6 (adds together). The v2.3 review noted v2.3 had 5 providers per the project's tech-stack.md — Manual Slop has 8 (per the qwen_llama_grok track); the count is independent of the abstraction. The token-cap awareness is NEW (v2.3 had byte-only rebuild triggers). v2.3 §5 ("the loop") is extended with a per-model token cap as a second rebuild trigger.
**Manual Slop implications:** Manual Slop's `src/ai_client.py` already has per-provider history locks (per `docs/guide_ai_client.md`) but does not have a per-model context-window table; the rebuild/compaction is currently driven by heuristic token estimates. The pattern "verify the window, don't guess; only assert what you've tested" maps to Manual Slop's `provider_state` architecture (per `docs/guide_ai_client.md`). The claude-code billing quirk (`env={"ANTHROPIC_API_KEY": ""}`) is a specific gotcha worth documenting — Manual Slop's claude-code integration (per tech-stack.md) may benefit from the same discipline.
**Decision candidate:** NEW Candidate 21 (MEDIUM). "Per-model token-cap awareness for Manual Slop `ai_client`": add `MODEL_CONTEXT_WINDOWS` table; rebuild fires on byte ceiling OR 0.85 of window; "don't guess" — omit rather than estimate. See `decisions.md` Candidate 21.
**Cross-refs:** §2 Conversation safety net (rebuild trigger gets a second condition); §3 Hooks (per-turn status can include `current model / window / usage`).
**Source-read citations:**
- `bin/helpers/nagent_llm.py:13-19``PROVIDERS` extended + `TOGETHER_BASE_URL` (bdfa2a6)
- `bin/helpers/nagent_llm.py:27-31``DEFAULT_MODELS["together"]` (bdfa2a6)
- `bin/helpers/nagent_llm.py:37-42``CREDENTIAL_ENV["together"]` = `("TOGETHER_API_KEY",)` (bdfa2a6)
- `bin/helpers/nagent_llm.py:54-77``MODEL_CONTEXT_WINDOWS` table (10 verified models) (bdfa2a6)
- `bin/helpers/nagent_llm.py:123-130``model_context_window(model)` returns `None` for unknown (bdfa2a6)
- `bin/helpers/nagent_llm.py:198-279` — Together client + `_together_chat` (always streamed) (bdfa2a6)
- `bin/helpers/nagent_llm.py:315-336``list_models("together")` — direct fetch because Together returns a bare JSON array (bdfa2a6)
- `bin/helpers/nagent_llm.py:381-400``list_providers()` — static catalog, no network (bdfa2a6)
- `bin/helpers/nagent_llm.py:582-625` — Together in `generate_text_with_usage` + `generate_with_upload_usage` (bdfa2a6)
- `bin/helpers/nagent_llm.py:739-770``_together_upload` — image-upload only, base64 data URL (bdfa2a6)
- `bin/helpers/nagent_llm.py:357-391``env={"ANTHROPIC_API_KEY": ""}` + error-result-survives-stream-exception + synthetic-error-text-skip (5075f6e)
- `bin/nagent:1075-1081``target = f"{llm.provider}/{llm.model}" if llm.model else llm.provider` (2edc7ee)
- `config.example.json:7``"context_window_tokens": 0` (bdfa2a6)
- `README.md:82-90` — providers table extension (bdfa2a6)
- `README.md:956-967` — "Conversation rebuilt (compacted...) when **either** trigger fires first" (bdfa2a6)
- `README.md:991-995``--list-providers` CLI example (bdfa2a6)
- `tests/test_nagent.py:1010-1042``test_call_llm_wait_spinner_names_provider_and_model` (2edc7ee)
- `tests/test_nagent.py:2734-2797` — 4 new claude-code tests (5075f6e)
**Honest gaps in this cluster:**
- `MODEL_CONTEXT_WINDOWS` is verified against the Together API only on 2026-06-17. Other providers' models are intentionally omitted. A future track should add more verifications.
- The `env={"ANTHROPIC_API_KEY": ""}` blanking assumes subprocess env takes precedence over inherited env. Correct on POSIX; Windows env handling could differ. Unverified.
- The Together `/v1/models` direct fetch at `bin/helpers/nagent_llm.py:315-336` is a vendor-specific workaround. If Together changes the response shape, the parser silently returns fewer models. A defensive check (count returned models, warn if zero) could harden this.
**Pattern deep-dive.** The provider-expansion abstraction is a four-piece composition: **register**, **window**, **trigger**, **bill**. Register: a provider is one tuple in `PROVIDERS` + one entry in `DEFAULT_MODELS` + one tuple in `CREDENTIAL_ENV` + one entry in `PACKAGE_HINTS`. The 5-tuple is enough to surface a provider in `--list-providers` and route a `generate_text_with_usage` call. Window: `MODEL_CONTEXT_WINDOWS` is a verified table, not an estimate. "Omit rather than guessed" (per `bin/helpers/nagent_llm.py:60-62`) is the discipline — the table at `bin/helpers/nagent_llm.py:54-77` lists exactly the models whose windows were verified by API error or by direct lookup, and the function `model_context_window` returns `None` for unknowns (the caller falls back to byte-only behavior). Trigger: rebuild fires on whichever trips first, the byte ceiling OR 0.85 of the model's window (per `README.md:956-967`). The 0.85 safety fraction is the data-oriented response to "model capability degrades under high context utilization, not just at the limit" (per the issues/0004 spec). Bill: the claude-code billing quirk (`env={"ANTHROPIC_API_KEY": ""}`) is the discipline "API-key billing stays the anthropic provider's job" (per `bin/helpers/nagent_llm.py:361-364`) — billing is data; the provider that owns the billing owns the env.
The token-cap awareness is the load-bearing change. A byte-only rebuild trigger is a proxy for token utilization, and the proxy fails on small-window models — `rebuild_at_kb: 384` is far too high to fire on a 8192-token model. The per-model window table is the data-grounded alternative. The `context_window_tokens` config key (per `config.example.json:7`) is the extension point: a user who wants a new model's window can add it without code change. The "unknown returns None" behavior at `bin/helpers/nagent_llm.py:123-130` is the discipline — a missing entry is not a default to a guess; it's a signal to fall back to the byte-only behavior, which is correct for large-window models and merely late for small-window models (the failure is visible, not silent).
The `bdfa2a6` commit message is explicit about the verification process: "DeepSeek-V4-Pro confirmed by a context_length_exceeded error ('maximum context length is 512000 tokens'). Qwen3.7-Plus/Max advertise context_length=1000000, but an oversized request is rejected with 'Range of input length should be [1, 983616]' — so the enforced input cap is 983616, with ~16384 of the 1M reserved for output." The distinction between "advertised total context_length" and "enforced input cap" is load-bearing — the table records the enforced cap, not the advertisement. This is the same data discipline as the project's `conductor/code_styleguides/cache_friendly_context.md`: stable data (verified numbers) vs volatile data (advertised numbers).
A code-shape sketch using survey grammar:
```
providers := { name: string, default_model: string,
credentials: [env-var], package: string,
context_window: int | nil } // [M] mutable aggregate
provider { name, model, env } :: LlmResult {ssdl} [B] // boundary
// SDK call; failures surface text + exit code
rebuild-trigger { conversation_chars, model, settings } :: fire? {ssdl} [I]
byte_trip := conversation_chars > settings.rebuild_at_kb * 1024
window_trip := model_context_window(model)
and tokens > window * CONTEXT_WINDOW_SAFETY_FRACTION
byte_trip or window_trip
```
The `{ssdl}` markers note the abstractions: the provider call is a boundary (B) where SDK errors become LlmResult errors; the rebuild trigger is an inspectable invariant (I) computed from data on disk.
## §6 Delegation rewrite