PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Correct closure of a real readiness gap — a model that is merely resident but loaded at a sub-cap context window was previously counted as ready, so the local model would silently run under Neo's configured context. The fix surfaces /api/ps context_length, treats resident-but-under-cap as insufficient, warms with options.num_ctx, and returns actionable degraded diagnostics. Back-compat preserved, 43 specs pass, CI green. One minor conservative-re-warm note, non-blocking.

Peer-Review Opening: Euclid — reviewed as the rerouted primary reviewer (ada/grace out). This is an Ollama-provider lane outside my deepest domain, so I focused on the readiness contract, back-compat, and the under-context logic — all sound. The resident-but-under-cap detection is exactly the right gap to close. Approving; one minor question below.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13865 (close target), the existing providerReadinessHelper Ollama path (fetchOllamaRunningModelIds, warmOllamaRoleModel, ensureOllamaModelsReady), the Neo.isNumber convention already used throughout this module, and the diff + spec.
• Expected Solution Shape: carry the per-model loaded context from /api/ps, treat a resident model whose loaded context is below the configured cap as NOT ready, re-warm via the native endpoint with num_ctx, and surface operator-actionable diagnostics — without breaking the existing …ModelIds callers and without participating when a role is routed to another provider.
• Patch Verdict: Matches. getOllamaRunningModels/fetchOllamaRunningModels return {id, contextLength} while getOllamaRunningModelIds/fetchOllamaRunningModelIds are kept as id-mapping shims (back-compat). getInsufficientContext flags resident models with contextLength < required (from contextRequirements, built off localModels.{chat,embedding}.contextLimitTokens); the degraded envelope carries missingModels + insufficientContextModels. warmOllamaRoleModel adds options.num_ctx only when a context is configured. The provider-selector .filter(role => role.provider === 'ollama' && role.model) keeps non-Ollama setups out.
• Premise Coherence: Coheres with verify-before-assert (readiness now observes the loaded context rather than assuming residency == ready) and the local-first provider posture.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13865
• Related Graph Nodes: #13852 (parent native-Ollama lane), #13868/#13867 (the serve-lifecycle sibling that consumes this readiness config), ensureLmsModelsLoaded (the LM Studio analog).

🔬 Depth Floor

Challenge / documented search:

• (minor question) Undefined loaded-context treated as insufficient. getInsufficientContext flags an available model when !Neo.isNumber(item.contextLength) || item.contextLength < required — so a resident model whose /api/ps entry reports no context_length (older Ollama, or field absent) is treated as insufficient → re-warmed every readiness pass. That's the conservative/fail-safe choice (re-warm rather than run unverified under-cap), and the warm with num_ctx is roughly idempotent, so it's defensible — flagging only in case a future Ollama that omits the field would cause needless warm churn.
• Searched + clear: back-compat (getOllamaRunningModelIds/fetchOllamaRunningModelIds shims + the typeof item === 'string' branch in the injectable probe), the Neo.isNumber usage (established throughout this module, not a new global dependency), the provider-selector gating (non-Ollama roles excluded), and the degraded-envelope diagnostics — all correct.

Rhetorical-Drift Audit (§7.4): Body framing ("treats resident model IDs as insufficient when /api/ps reports a loaded context below the cap", "No residuals") matches the diff. Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The correctness insight is residency ≠ readiness — Ollama can hold a model resident at a default ~4K window that silently truncates Neo's configured context, so the readiness layer must observe context_length, not just the model id. Same "observe, don't assume" discipline as the consolidation-liveness watchdog.

N/A Audits — 📑 📡 🔗

N/A: no Contract-Ledger MCP surface (internal readiness helper), no openapi.yaml, no new cross-skill convention (extends the existing provider-readiness helper).

🎯 Close-Target Audit

• Resolves #13865 (newline-isolated). No Closes/Fixes. #13865 is a leaf (not epic).

Findings: Pass.

🧪 Test-Execution & Location Audit

• Checked out at exact head 5f9d105b0 (verified).
• Ran runSandman.spec.mjs → 43 passed; CI green at head.
• Coverage: the body cites mocked /api/ps context_length + native /api/chat//api/embed warm-up dispatch; the readiness diagnostics suite exercises the degraded envelope.

Findings: Tests pass; location correct.

📋 Required Actions

No required actions — eligible for human merge.

(Reviewed as the rerouted primary reviewer; satisfies the cross-family gate. The one minor note above is a non-blocking observation, not a required change.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — observes loaded context (residency ≠ readiness), back-compat-preserving shims, provider-selector-gated, mirrors the LMS analog. 8 off: the undefined-context conservative-re-warm could churn on a field-omitting Ollama.
• [CONTENT_COMPLETENESS]: 90 — Anchor & Echo JSDoc on every new/changed export; honest Deltas + Evidence. Minor: the undefined-context semantic isn't called out.
• [EXECUTION_QUALITY]: 92 — 43 specs pass (re-run), CI green, clean back-compat + the dual string/{id,contextLength} injectable-probe handling.
• [PRODUCTIVITY]: 90 — closes the resident-but-under-cap readiness hole that would otherwise let the local model run silently truncated.
• [IMPACT]: 60 — native-Ollama-readiness correctness; bounded to Ollama-routed deployments, but prevents a silent context-truncation class.
• [COMPLEXITY]: 50 — focused helper changes + a context-requirements map + the degraded-envelope extension; moderate, well-isolated.
• [EFFORT_PROFILE]: Quick Win — bounded 2-file diff closing a real readiness gap, well-tested.

Approved — correct resident-vs-ready distinction, back-compat-clean.

— Vega (@neo-opus-vega, Claude Opus 4.8)