Harden first-run: an honest exit code, a 12x faster first ingest, and two storage guards - #11
Merged
Merged
Conversation
…n.env` `.gitignore` had `.env`, a literal path rather than a glob, so a `neon.env` holding a live Postgres password and S3 secret keys sat untracked but unignored -- one `git add .` from entering history. Verified with `git check-ignore -v`: no match before, `.gitignore:15:*.env` after. `.env.sample` is tracked and the glob does not match it, so it stays tracked. First in the series on purpose: every later `git add` here is only safe once this rule is in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing nothing
`cmd_ingest` printed its stats dict and returned 0 unconditionally, never
reading the `embedder_down` that `run_ingest` latches. MEASURED against a dead
backend: `{'added': 0, 'skipped': 0, 'failed_feeds': 1, 'embedder_down': True}`
and exit 0. The control is measured too, because a careless fix breaks it:
against live Ollama, `{'added': 1343, 'skipped': 1, 'failed_feeds': 0}`, exit 0.
That 0 was believed in two places. install.py's `step_first_data` shells out to
`attest ingest`, and the generated hourly refresh script tests
`if uv run attest ingest`. So a user whose Ollama was not running got an empty
feed, an install that reported success, and a cron job logging success
indefinitely -- silent, self-confirming failure on first run.
`cmd_tag` five lines below already did this correctly for `chat_down`; this
copies that shape. A partial run still exits 1: once the latch trips
`run_ingest` stops, so the remaining feeds were never attempted -- a degraded
run, not a clean one.
Also threads `need_vectors=True` through this path's `open_db` (the sqlite-vec
guard in the following commit needs it), and fixes three test doubles whose
`run_ingest` lambdas predated main's `clients` kwarg: they raised TypeError
once `cmd_ingest` began passing it, a break that merged textually clean and
surfaced only under test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n not 20.8
`EmbeddingClient.embed` sent one string per HTTP round trip and `run_ingest`
called it from a plain list comprehension, so a first ingest paid one request
per item. The OpenAI-compatible `/embeddings` endpoint already accepts a list.
MEASURED on 96 real corpus items (titles + summaries from a 1343-item
database, not a fixture) against live Ollama and embeddinggemma:
serial 39.95s = 416.2 ms/item
batched 4.04s = 42.1 ms/item 9.9x
identical vectors in identical order: True
For a ~3000-item first ingest that is 20.8 min -> 2.1 min, which is the single
largest cost in getting a new user to a ranked feed.
`embed_many` sorts the response by each element's `index` before returning.
The OpenAI contract does not guarantee `data` comes back in request order, and
mismatching vectors to items would corrupt the index in a way nothing else
would notice -- hence the order assertion in the measurement above and a test
that feeds a deliberately out-of-order stub response.
The embedder-down latch is preserved deliberately: `embed_documents` raises
straight out of the chunk loop exactly as the per-item call used to, so
`run_ingest`'s handler still classifies it and still stops after one honest
message rather than 22 tracebacks. A chunk that fails mid-feed abandons that
feed's whole embedding pass and never reaches the write transaction -- the
same all-or-nothing per feed as before. No partial-chunk salvage was added;
that would be a behaviour change rather than a preservation.
EMBED_BATCH_SIZE is 16. A live sweep on the same texts found 32 at 45.4
ms/item and 96 at 40.8 ms/item, so there is ~17% left on the table, but the
failure unit grows with the chunk and 16 already delivers the speedup that
matters. Left as a one-line follow-up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…internal error"
`rank.py` raised a carefully worded `EmbedderUnavailable` and the web UI
rendered it well, but the MCP path turned it into `"internal error in
list_feed; see server logs"` -- so the two most likely agent first-contacts,
`feed.list` and `feed.search`, told the caller nothing it could act on. Per
CLAUDE.md an unreachable model server is an expected, actionable refusal, so it
belongs in the ToolError category rather than the bug path.
MEASURED against a dead LLM_BASE_URL, what an agent now receives:
ok=False, message='embedding model unreachable
(LLM_BASE_URL=http://127.0.0.1:9/v1) -- is ollama running?
(`attest install --check` diagnoses this)'
`_tool.py` imports the type lazily. Doing it at module scope pulls sklearn
through `rank.py` and costs ~929ms on every MCP tool call, which
`test_cli_help_stays_fast` exists to catch.
Includes a cross-file fix this change caused, and which is the reason the
pytest gate is not optional: `library.py:_semantic` caught
`(httpx.HTTPError, OSError)` to degrade to substring search, and
`EmbedderUnavailable` is a plain RuntimeError, so it silently stopped matching
and escaped uncaught. Repo-wide `ruff check` and `ty check` both passed while
that was broken -- a RuntimeError ceasing to match an except tuple in another
module is invisible to a linter and a type checker. Only a test found it.
Both policies are correct and both are kept: feed.* SURFACES the condition to
the agent, library search DEGRADES to substring per `_semantic`'s own
docstring ("None when the wire could not be reached"). Same failure, two
legitimate readings, so the type appears in both places. The import there is
function-local too, matching that file's existing idiom.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An empty result returned `[]` with `ok: true` whether the database held
nothing at all or simply nothing in the last 14 days. Those are different
problems with different fixes, and an agent could not tell them apart -- the
documented worst case for a new user, who sees an empty feed and no reason.
MEASURED on a fresh database, what a caller now gets:
ok=True, n_items=0,
message='0 item(s) -- the database has no items yet; run `attest ingest`
to fetch some'
`ok: true` is deliberate: an empty feed is a legitimate answer, not a refusal,
so the explanation rides in the existing `message` field rather than raising.
The existence check lives in a new `rank.item_count()` reader rather than a raw
`SELECT COUNT(*)` in the MCP layer, so MCP_SQL_BASELINE stays at 21. Main
reverted that same ratchet 22 -> 21 in cbac191, whose message calls a raw
SELECT up here "the exact pattern MCP_SQL_BASELINE exists to push back on";
raising it again would have silently reversed a decision made four commits
later, and the domain reader is what that commit demonstrates instead.
`rank.py` also gains the `EmbedderUnavailable` raise sites for the previous
commit: `vector_search` now converts `httpx.ConnectError`/`ConnectTimeout`
into it, which is what `feed.search` needed -- a raw transport error used to
escape and never reach the handling that already existed. The except clause is
narrow on two concrete types, so it needs no BLE001 suppression and the
inventory stays at 7. The warm-cache-serves-stale policy is untouched: a down
embedder with a cached vector still degrades rather than raising, and only the
genuinely cold case raises.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rests string With no clicks `blend_weight(0)` is 0.000, so the classifier never fires and the whole first session is cosine similarity between one free-text interests string and each item. The onboarding form asked for that string with an empty textarea and a placeholder, which makes the blank box the product on day one. Three archetypes (researcher, bench-chemist, ml-engineer) are now offered on the web form and from `feed.persona_suggest_interests`. They come from the existing `db.SEED_USERS` rather than new prose, and the text is passed through VERBATIM: CLAUDE.md records that repetitive interests text embeds BETTER than hand-tightened text (0.588 vs 0.568 mean similarity), because the string IS the profile vector, so tidying it for display would make it worse. Nothing is autoseeded. Clicking a template prefills the textarea, still fully editable; a persona exists only when someone submits. MEASURED on a fresh empty database: `GET /onboard` returns 200 with all three rendered and `personas created: []`. That property is load-bearing -- this repo already had the failure where `/` opened the author's persona and created it for whoever arrived. The tool side closes a real hole: `_propose_interests` previously offered only the corpus's most common tags, and `item_tags` is empty until the model tagging pass runs, so it had nothing to suggest exactly when a new user needed it. On an empty corpus it now returns `prevalent_tags: []` plus the three archetypes, 800 characters against a 7000 ceiling. The buttons carry `data-interests` and read `this.dataset`, not `| tojson` into an inline `onclick`. Jinja's tojson only JSON-escapes, so a `"` in the text breaks out of the HTML attribute -- verified live, producing `onclick="x = "he said \"hi\"""`. Same class of bug as this repo's `hx-vals` scar, where a reader named `ann\` silently broke both vote buttons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on without loadable extensions
Two storage guards, plus the count update their arrival forces.
MIGRATION 010, `embedding_model`: one row per vec0 table naming the
EMBED_MODEL that produced its vectors. `_ensure_vec_tables` already refused a
DIMENSIONALITY mismatch, but model identity was recorded nowhere --
`item_features.model` names the TAGGING model only. So a database embedded
with embeddinggemma, opened by a user configured for a different 256-dim
model, passed the dims guard and produced silently incoherent similarity: same
width, different vector space, no error. MEASURED, the refusal now fires:
RuntimeError: database has item_vectors vectors recorded from
model='embeddinggemma' but EMBED_MODEL=some-other-256d-model
-- re-ingest into a fresh database or set matching EMBED_MODEL
Legacy databases are never refused: vectors with no recorded model get the
configured EMBED_MODEL backfilled as the row of record on first open, because
there is no way to recover what actually produced old vectors and refusing
would break every existing user. The consequence is first-open-wins, which
matters for any database we ever DISTRIBUTE -- a shipped file must arrive with
`embedding_model` already populated or the guard is disarmed for exactly the
users it protects.
Numbered 010, not 009: main's research branch claimed 009 concurrently. The
ladders interleave rather than collide, so the merged ladder is
`...(8), (9, research), (10, embedding_model)`.
THE EXTENSION GUARD: `get_db` called `enable_load_extension` unconditionally.
On a Python built without `--enable-loadable-sqlite-extensions` that attribute
does not exist, and `AttributeError` is neither OSError nor sqlite3.Error, so
it escaped `open_db`'s filter and killed EVERY subcommand -- including
`attest runs scan` and `attest claims`, the model-free tier the README leads
with. This was not hypothetical: a bare `3.12` in `.python-version` bound a
pyenv build lacking that support and broke the whole tool while CI, which uses
`actions/setup-python`, stayed green.
Policy follows CLAUDE.md verbatim -- "the ledger and claim checking need no
model; the feed does". `_ensure_vec_tables` returns early instead of
attempting `CREATE VIRTUAL TABLE ... USING vec0`; the relational schema and
the whole ladder still run. `open_db` gained `need_vectors`, true on the six
call sites that actually query a vector table, because `library.*` and
`ingest` query them unconditionally even on fielded-only paths -- so merely
leaving the tables absent would surface as an obscure `no such table` deep
inside those modules rather than an actionable refusal.
COUNTS: main reached 28 sqlite_master / 17 application via `reference_fulltext`
(009); `embedding_model` makes it 29 / 18. Re-measured on the merged tree
rather than reasoned about, and written into CLAUDE.md with both pinning tests
updated -- `test_db.py` and `test_library_db.py` police the same claim from
opposite directions, and when 010 first landed one was updated and the other
was not, which is exactly the drift they exist to catch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…all cost is disclosed THE DOCTOR: on a clean HOME with no hermes binary, `attest install --check` reported BROKEN for `skill_copy` and so exited 1, even though the other three agent-wiring steps already degraded to SKIPPED via `_find_agent_binary()`. `step_skill_copy` never took an `agent` parameter and always ran. It now skips without one, matching its three siblings, so a user who wants only the local feed and ledger gets a clean exit. Verified both ways: with no agent and a seeded database `--check` exits 0; with an empty database only `first_data` is BROKEN and it exits 1, which is a real local gap and should fail. A premise I was given for this work turned out to be FALSE and is recorded here rather than quietly dropped: `--check` was said to create `~/.hermes/` because a `mkdir` preceded the check guard. It does not -- the guard is correctly ordered, as are all four other mutation sites. The `~/.hermes/` creation came from invoking the real `hermes` binary during read-only `hermes mcp list`/`cron list` calls. Reproduced with a no-op stub: zero mutation. The regression test was added anyway, snapshotting the whole fake home tree before and after, since it would catch the bug the premise described including an empty-directory variant a file listing would miss. THE DISCLOSURE: install.md said models "are pulled for you" with no size. MEASURED: gemma4:e2b-it-q4_K_M 7.2 GB + embeddinggemma 621 MB = ~7.8 GB of download, ~5.4 GB held resident while warm. Ollama is now named as a separate prerequisite with its download link, and the no-model tier is stated FIRST -- the run ledger and claim checker need neither, reproduced from a clean clone with the backend unreachable in 2.2s, so a reader who only wants provenance can skip the 7.8 GB entirely. The documented `ollama >= 0.32.9` floor is marked as a manual requirement, because nothing in this repo checks it. `.python-version` pins `3.12.12`, the exact patch, not a bare `3.12`. A bare minor version is a lookup, not a pin: uv resolved it to a local pyenv 3.12.4 built without loadable sqlite extensions, which broke every command while passing CI. uv-managed 3.12.12 and 3.13.11 both have the support; this keeps CI's 3.12 leg honest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t still small
The batching commit left EMBED_BATCH_SIZE at 16 because that was the width the
first measurement happened to use, and said so. Swept it properly on 96 real
corpus items against live Ollama/embeddinggemma, best of three runs each:
16 -> 57.6 ms/item (6 requests) the original choice
32 -> 47.5 ms/item (3 requests) +21.4%
48 -> 43.5 ms/item (2 requests) +32.6%
64 -> 45.5 ms/item (2 requests) +26.6%
96 -> 42.3 ms/item (1 request) +36.1%
Per-item time flattens after about 32 -- 48 and 96 differ by roughly what two
runs of the same width differ by, and 64 came in slower than 48 -- while the
cost of a failure grows linearly with the chunk, since a chunk fails or
succeeds together and `_embed_entries` raises out of the loop, abandoning that
feed's whole embedding pass. So this is a blast-radius choice, not a hunt for
the minimum.
The endpoint imposes no ceiling worth designing around: 256 texts in a single
request returned 256 vectors at 40.7 ms/item, so nothing here is working
around a server limit.
32 takes the bulk of the remaining gain and keeps the unit of loss small.
Verified at the new width: a dead backend still latches and exits 1 with one
message rather than one per feed, and a live run added 1363 items, exit 0.
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.
Nine commits aimed at one question: what does a researcher hit in their first
five minutes, and does it lie to them? Every number here is measured against
live Ollama/embeddinggemma on real corpus items, not fixtures.
What a new user gets
[]withok: true"internal error in list_feed"The two that matter most
attest ingestexited 0 after ingesting nothing.cmd_ingestnever read theembedder_downthatrun_ingestlatches. That 0 was believed byinstall.py'sstep_first_dataand by the generated hourly refresh script, so a user whoseOllama was not running got an empty feed, a successful install, and a cron job
logging success indefinitely. Silent, self-confirming failure on first run.
Batched embedding, measured 9.9x. 416.2 -> 42.1 ms/item on 96 real items,
then 47.5 ms/item at the widened chunk.
embed_manysorts the response by eachelement's
index, because the OpenAI contract does not guaranteedataorderand mispairing vectors with items would corrupt the index invisibly. The
embedder-down latch is preserved: one honest message, not 22 tracebacks.
Storage guards
Migration 010 records which
EMBED_MODELproduced each vec table's vectors.Dimensionality was already guarded; model identity was recorded nowhere, so a
same-width different-model reopen produced garbage similarity with no error.
Legacy databases are never refused -- the configured model is backfilled on
first open -- which means first-open-wins, so any database we ever distribute
must ship with
embedding_modelalready populated or the guard is disarmed forexactly the users it protects.
The sqlite-vec extension load no longer kills the tool on a Python built
without
--enable-loadable-sqlite-extensions. This was not hypothetical: a bare3.12in.python-versionbound a pyenv build lacking that support and brokeevery command while CI stayed green, because
actions/setup-pythonbuilds dohave it. Hence the exact-patch pin. Policy follows CLAUDE.md verbatim: the
ledger and claim checker need no model, so they keep working.
Notes for review
MCP_SQL_BASELINEstays at 21. The empty-state check needed aCOUNT(*);rather than raise the ratchet main deliberately reverted in cbac191, it moved
into a
rank.item_count()domain reader, which is what that commitdemonstrates.
tree. Main reached 28/17 via
reference_fulltext(009);embedding_model(010) makes it 29/18. Both pinning tests and CLAUDE.md updated together --
they police the same claim from opposite directions and had already drifted
apart once.
--checkfilesystemmutation I was asked to fix does not exist; the guard is correctly ordered and
the
~/.hermes/creation came from the realhermesbinary during read-onlycalls. The regression test landed anyway.
vector_searchraiseEmbedderUnavailablesilently stopped it matchinglibrary.py's(httpx.HTTPError, OSError)fallback. Repo-wideruff checkandty checkboth passed while that was broken. Only pytest found it.
Verification
Full gate green on the pushed tree, all eight hooks, read line by line rather
than trusted from the exit code -- an earlier run exited 0 while the complexity
hook had failed inside it:
Full suite 1841 passed, 2 skipped, 3 deselected.
🤖 Generated with Claude Code