fix: the documented first run could not reach a running server - #201
Merged
Conversation
Three defects found by running `init` → `ingest` → `serve` against real
content for the first time. Each of them breaks a new user before they see
anything work.
**`ingest` and `serve` chose different embedders.** `--embedding-model` was a
fixed typer default (`ollama-local/nomic-embed-text-v2-moe@2026-04`) that never
consulted the environment, while the API calls `resolve_embedding()`, which
selects OpenAI whenever `OPENAI_API_KEY` is set. So on README.md:214's own path,
any user with an OpenAI key — including this repo's .env — got:
LanceDB table 'chunks' was built with embedding model
'ollama-local/nomic-embed-text-v2-moe@2026-04', but the active embedder is
'openai/text-embedding-3-small'.
The guard is right; vectors from two embedders are not comparable. The defect
was that two entry points to one product chose differently. `_kb_embedding()`
now resolves the same way the API does, and every KB command routes through it
— all six, plus `_harness_dispatch`, because fixing `ingest` alone would have
created a fresh `ingest` ↔ `kb-search` mismatch. `--embedding-model` still pins
the reference and embeds through Ollama, exactly as before.
**`ingest` silently ingested nothing when any path segment started with a dot.**
`discover_files` tested every segment of the *absolute* path, so an ancestor
named `..` or `.staging` rejected the entire tree. `opspilot ingest ../docs`
reported "0 succeeded · 0 failed" — indistinguishable from an empty directory.
The filter now tests only the segments below the walked root, which is what its
docstring always described: skip VCS metadata and swap files *inside* a tree.
**Bundle import pointed at a command that does not exist.** It printed "run
`opspilot kb ingest`"; there is no such subcommand. The flow broke on the very
next step after an import.
Verified on the exact scenario that failed: with OPENAI_API_KEY set and no
OPSPILOT_EMBED_PROVIDER, `ingest` records `openai/text-embedding-3-small`,
`serve` starts, and `kb-search` returns hits.
Noted, not changed: `opspilot run` used one `make_provider("ollama-local")` as
both its embedder and its *chat* provider, so it ignores the playbook's model.
That is pre-existing and out of scope here; the line is left exactly as it was,
with a comment saying so.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI caught what a machine with Ollama running could not: six tests in test_cli_ingest.py passed locally and failed on the runner. The fixture patched `opspilot.cli.make_provider`, but the CLI's default embedder now resolves through `embedding.resolve_embedding` — which is the point of the change — so the stub no longer intercepted it, and with no Ollama and no OPENAI_API_KEY the resolver correctly returned an embedder that raises. The fixture now patches both targets, since an explicit --embedding-model still builds its provider in `opspilot.cli` while the default goes through `opspilot.embedding`. `_MockProvider` grows `health_probe`, which the resolver calls before selecting Ollama, and the fixture pins OPSPILOT_EMBED_PROVIDER so a developer machine that exports OPENAI_API_KEY does not send the suite to a paid API. Reproduced the runner's environment locally — no keys, Ollama pointed at a dead port — to check rather than assume: 6 failures became 1, and that one (test_load_config_invalid_yaml_falls_back) fails identically on main under the same fake URL, so it is the simulation, not this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vicenteliu
added a commit
that referenced
this pull request
Aug 19, 2026
The Memory / Consultation / Working-set / distillation / bundle stack was driven end to end for the first time on 2026-08-19, after being built and unit-tested in a single session. Nine defects, all fixed in #201–#205. The list is in ROADMAP's Open section, but the reason to write it down is not the list. Every one lived in a seam no single test owns — two entry points disagreeing on a default, a path filter's scope, a sentence missing from a prompt — while the suite stayed green at 1311 tests. Three predictions written before the run scored 0/3, because they covered what had already been thought about and the defects were where nothing had. Also records the two gates that caught what a developer machine could not (CI has no Ollama; the behaviour gate scored a new prompt at 1/3), the one decision left open on purpose — whether kb/retrieval.py joins the gate's protected paths — and that the path has not been re-run since the fixes landed. The behaviour gate section now says five behaviours, not four: #205 added the assistant offering a standing fact to Memory, in two halves. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
vicenteliu
added a commit
that referenced
this pull request
Aug 19, 2026
* fix: an exhausted tool loop answers, instead of echoing its own preamble Found by re-running the end-to-end path after #201-#205. Asked "How do I renew a TLS certificate before it expires?", deepseek-v4-flash ran `kb_search` six times — three of them the identical query — hit CHAT_MAX_TURNS, and the user got 62 characters: Let me check the knowledge base for any relevant procedures. with seven citations attached to it and 578 output tokens billed. The loop's exhaustion branch returned the last round's `resp.content`. But the last round produced a *tool call*, so that content is the preamble the model writes before reaching for a tool — never an answer. The comment said "answer with whatever the last turn produced"; the last turn produced a tool call. The cap now bounds the *tool* rounds. When they run out, one more round goes out with no tools: the results are already in `provider_msgs`, and with nothing left to call, the only move is to answer from them. Measured on the same question, same forced condition, live models: before 73 chars "Let me search the knowledge base for TLS certificate…" after 1561 chars the certbot / DigiCert renewal procedure, 5 citations The final prompt is *rebuilt*, not extended. `system_prompt` tells the model to call kb_search before answering, to call report_conflict *before it answers*, and which skills it may load — three instructions it can no longer follow, and the conflict one is a precondition it would be stuck on. PROPOSAL_HINT survives, because offering a fact to Memory is prose rather than a tool call. The instruction rides the system prompt rather than an appended user turn: `role="tool"` renders as a `tool_result` block inside a *user* message on Anthropic, so appending one more user message would stack two in a row. Verified against live claude-haiku-4-5 and deepseek-v4-flash — both return a full grounded answer through the exhaustion path. The model repeating a query it has already run is a separate, cheaper problem. It no longer costs the user an answer. behaviour-gate: 6 passed — memory injection 3/3, conflict reported 3/3, distillation keeps dead ends 3/3, proposals stay read-only 3/3, memory proposal 3/3, memory proposal restraint 3/3 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: one sample directory fed two commands, and ingest lost README.md:214 tells a new user to run opspilot ingest examples/sample_data_en/kb/ On the repo's own sample data that reported `10 succeeded · 5 failed`. The five failures were `chunks.jsonl` files raising `AdapterError: unsupported file type`, and worse, the five `doc-meta.json` sidecars were ingested *as knowledge documents* — 5 of the 17 chunks in a fresh KB were JSON metadata. Two of them came back in the top five for "why would a pod be stuck in CrashLoopBackOff". Neither half is a bug in isolation. That directory was built in 545ae7e as the sample input for `opspilot kb load-dir`, which recursively loads doc-meta.json + chunks.jsonl pairs. The README pointed a second, different command at the same tree later. One directory, two commands, both correct on their own, never run against each other. Split it: source documents stay in `kb/`, their frozen projections move to `fixtures/`. No product code changes, and no README changes either — the command it already documents is now the one that works. ingest examples/sample_data_en/kb/ 5 succeeded · 0 failed · 12 chunks kb load-dir examples/sample_data_en/fixtures/ 5 pairs, ids chk_f3a40001… The same search that used to surface metadata at ranks 3 and 5 now returns SOP prose in every position. `fixtures/README.md` says why they live apart, because the obvious tidy-up is to move them back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: retrieval joins the behaviour gate, and ROADMAP stops lying twice **The open decision is closed.** #206 left one deliberately: whether `kb/retrieval.py` belongs on the behaviour gate's protected paths. It does. #203 shipped without gate evidence, and what it changed was which chunks reach the model at all — the input every one of the five prompt-driven behaviours is judged on. The list's own rule already settled it: over-triggering costs minutes, missing the change costs the reason the gate exists. The entry carries that reasoning, because retrieval.py holds no prompt and the next reader will ask why it is there. **Two counts were stale.** #205 added the fifth behaviour and neither the CI comment nor the Makefile banner followed; CI also printed a paste-me example reading `behaviour-gate: 4/4 passed (votes 3/3, 3/3, 3/3, 3/3)`. The gate only greps for the `^behaviour-gate:` prefix, so a contributor copying that example would have landed a permanent "4/4" for a six-case run — in the one artifact the comment above it calls the whole point. **ROADMAP described two shipped things as open.** #175's silent model swap was fixed in #177 (`model_fallback` trace event, result re-labelled), and the proposed-actions preview/execute UI shipped in #190. Both still read as outstanding work. The real gap in proposed actions is elsewhere and now says so: nothing in `playbooks/` opts in, so an escalated Session returns `{"actions": []}` on every fresh install, and outside ROADMAP the key is named nowhere — not in a playbook, not in ADR-0028, which says only that playbooks opt in. **And the second run is recorded.** Ten checks against the nine fixes from #201-#205, ten passes. Three further defects, two fixed in this PR, plus one rough edge: the CLI writes as `cli:<osuser>` while the loopback API writes as `local-dev`, so `opspilot workingset status` reports nothing open while the web UI has a set open for the same person. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Found by running
init→ingest→serveagainst real content for the first time. Three defects, each of which stops a new user before anything works.1.
ingestandservechose different embedders — serve refused to start--embedding-modelwas a fixed typer default that never consulted the environment; the API callsresolve_embedding(), which selects OpenAI wheneverOPENAI_API_KEYis set. On README.md:214's own path, any user with an OpenAI key hit a hard refusal.The guard is correct — vectors from two embedders are not comparable, and it says so well. The defect is that two entry points to one product chose differently.
_kb_embedding()now resolves the same way the API does. Applied to all six KB commands and_harness_dispatch(the six harness commands' single funnel), because fixingingestalone would have created a freshingest↔kb-searchmismatch.--embedding-modelstill pins the reference and embeds through Ollama.2.
ingestsilently ingested nothing when a path segment started with a dotdiscover_filestested every segment of the absolute path:so any dot-prefixed ancestor rejected the whole tree. Isolated on a fresh home each time, same 5-file corpus:
.../e2e/../e2e2/.../kb.../e2e2/.../kbOPSPILOT_HOMEcontaining...../.hidden-corpus(named explicitly)opspilot ingest ../docs/runbookswas a silent no-op, and0 succeeded · 0 failedreads as "nothing to do". Now filtered relative to the walked root — which is what the docstring always described.3. Bundle import pointed at a command that does not exist
It printed
run `opspilot kb ingest`after a successful import. There is nokb ingestsubcommand; the working command isopspilot ingest.Verification
On the exact scenario that failed —
OPENAI_API_KEYset,OPSPILOT_EMBED_PROVIDERunset:ingestrecordsopenai/text-embedding-3-small(wasollama-local/...)servestarts (was a hard refusal)kb-searchreturns hitsBoth previously-silent ingests now report
5 succeeded · 10 chunks. New tests: 3 intest_ingestion.py(dot-directory,..segment, and that.git/inside a tree is still skipped) and 5 intest_embedder_agreement.pylocking the CLI-to-API invariant across the four key/request combinations.pytest -m "not slow and not requires_ollama and not requires_api_key"— 1319 passed. No protected path touched.Noted, not changed
opspilot runused a singlemake_provider("ollama-local")as both its embedder and its chat provider, so it ignores the playbook's model. Pre-existing and out of scope; the line is left as it was, with a comment recording it.🤖 Generated with Claude Code