Skip to content

fix(encoder): the provider-unavailable embed skip warns, and then throttles itself - #216

Merged
acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/embedder-failure-inconsistent-log-level
Sep 7, 2026
Merged

acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/embedder-failure-inconsistent-log-level

Conversation

@RobertSigmundsson

Copy link
Copy Markdown
Contributor

Summary

  • The provider-unavailable branch of the inline embedding path now logs at WARNING with the smem reindex hint, matching the TimeoutError branch beside it, instead of at DEBUG with nothing.
  • It throttles itself: the first occurrence and every hundredth warn in full, the rest stay at DEBUG with a running count.
  • Adds three tests over the level, the hint, and the throttle interval.

Why

_embed_created_neurons has two failure branches that end in the same place: the neuron is saved keyword-only and smem reindex back-fills the vector later. The TimeoutError branch says so at WARNING. The provider-unavailable branch — the more common of the two, since a provider being down outlasts any single request — said nothing at DEBUG, invisible under any default configuration. The first sign of trouble was recall quietly getting worse.

Raising it to WARNING and stopping there would trade a silent failure for a loud one, because the write path encodes in a loop: smem train, smem train-db and smem_remember_batch all push many neurons through this branch per invocation, and each WARNING carries a traceback.

Measured over 200 consecutive failures against a provider that raises, counting records and formatted bytes:

WARNING bytes at WARNING DEBUG total bytes
main today 0 0 200 156,600
WARNING with no throttle 200 175,400 0 175,400
this PR 3 2,728 197 22,913

The middle row is the version of this fix I am not proposing. Under a default configuration the operator sees the first column: nothing at all today, roughly 171 KiB of tracebacks under the naive fix, and 2.7 KiB here — visible, and bounded.

Changes

  • engine/encoder.py: a module-level counter for the branch; WARNING with exc_info and the reindex hint on occurrence 1 and every 100th, DEBUG with the running count otherwise. Both messages carry the number of neurons affected and the occurrence number.
  • tests/unit/test_encoder_provider_unavailable.py (new): the level and hint, that three failures produce one WARNING and two DEBUG, and that the warning recurs at occurrence 100.

A simplification worth naming

The counter is process-wide and is never reset. If a provider goes down, recovers, and goes down again inside one long-lived process, the second outage warns at its hundredth occurrence rather than its first. Resetting it properly means teaching the encoder what "recovered" means, which is more state than this fix should own, and the every-hundredth rung still surfaces the outage. Happy to revisit if you would rather it tracked recovery.

Test plan

  • pytest tests/unit/test_encoder_provider_unavailable.py — 3 passed.
  • With engine/encoder.py reverted to main and the tests kept, all three fail — they measure this branch rather than themselves.
  • The table above, measured with a stub provider that raises and a handler capturing formatted records including tracebacks.
  • pytest tests/ -m "not stress" -n 4 against a live SurrealDB v3.2.0 — 7286 passed, 48 skipped, 1 xfailed, which is main's 7283 plus the three tests added here. Two tests in tests/unit/test_dashboard_brains_scope.py fail on this branch and on main alike: they want a live database and collide with one another under -n. Both pass when that file is run on its own.
  • ruff check src/ tests/ clean; ruff format --check src/ tests/ reports 740 files already formatted.
  • mypy src/ --ignore-missing-imports — success, no issues found in 354 source files.
  • Coverage under the CI gate: 72.37%, against 72.36% on main.
  • CHANGELOG.md untouched — left to the release entry, as with fix(storage): bind datetimes in time comparisons so they select by value #191fix(memory): refresh content-derived fields on compress, restore, and refine #193.

Verified by

@RobertSigmundsson

…ottles itself

The inline embedding path has two failure branches that end identically: the
neuron is saved keyword-only and `smem reindex` is the way to back-fill the
vector. The TimeoutError branch logged that at WARNING with the hint. The
provider-unavailable branch logged at DEBUG, with no hint — invisible under
any default configuration, for what is the more common of the two failures.
The operator's first sign of trouble was recall quietly getting worse.

Raising it to WARNING alone would trade a silent failure for a loud one. The
write path encodes in a loop — `smem train`, `smem train-db` and
`smem_remember_batch` all run many neurons through it per invocation — so one
WARNING with a traceback per neuron turns a provider outage into a log flood
of roughly a kilobyte per record.

So the branch warns on the first occurrence and every hundredth after that,
with the traceback and the reindex hint, and keeps the ones in between at
DEBUG with a running count. A down provider is now impossible to miss and
costs a bounded number of log records rather than one per neuron.

The counter is process-wide and is not reset when the provider recovers, so a
second outage in the same long-lived process warns on its hundredth
occurrence rather than its first. That is a deliberate simplification: the
alternative is state that has to know what "recovered" means, and the
every-hundredth rung still surfaces the outage.

tests/unit/test_encoder_provider_unavailable.py covers the level, the hint,
the first-occurrence warning and the throttle interval. It pins the fail-soft
contract from the encoder's side rather than the storage's: the stub storage
asserts that update_neuron is never called when the embed failed, so the branch
cannot start writing rows it has no vector for.

@acidkill acidkill left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff. Provider-unavailable now warns like the timeout branch, throttled to first + every 100th so a down provider cannot flood the log.

@acidkill
acidkill merged commit 3a6a02e into acidkill:main Sep 7, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants