Skip to content

fix(compression): the brain pre-fetch logs its failure instead of degrading in silence - #212

Merged
acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/compression-brain-prefetch-logs-the-failure
Sep 7, 2026
Merged

acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/compression-brain-prefetch-logs-the-failure

Conversation

@RobertSigmundsson

Copy link
Copy Markdown
Contributor

Summary

  • The once-per-pass brain pre-fetch in CompressionEngine.run now logs a warning with the cause when it fails, rather than falling back in silence.
  • Adds two tests: the failing fetch must warn and the pass must still complete; a successful fetch must stay quiet.

Why

This is follow-up 2 from the review on #193, which described the problem better than I can paraphrase: the pre-fetch is "the only unlogged except" in a file where every other one logs, and although it is "correctly fail-soft — the helper re-fetches per fiber", two consequences are invisible.

The first is a performance cliff with no signal. The whole point of the pre-fetch is one brain lookup per pass instead of one per fiber; when it fails, the pass quietly reverts to the per-fiber shape it was written to avoid, and nothing in the log accounts for the slowdown.

The second is worse, because it misdirects. If the storage problem persists, the operator does eventually see a warning — but it comes from content_refresh and blames the embedding provider. The actual fault is a brain fetch, in a different module, that said nothing.

A logger.warning(..., exc_info=True) before the fallback fixes both. It changes nothing about when any code runs.

Changes

  • engine/compression.py: a logger.warning with exc_info=True in the except around the brain pre-fetch, before brain = None, and a comment recording why the branch stays fail-soft.
  • tests/unit/test_compression_brain_prefetch_logging.py (new): test_failed_brain_prefetch_is_logged swaps get_brain for a coroutine that raises and asserts both the warning and that run still returns a report; test_successful_brain_prefetch_stays_quiet is the positive control.

Test plan

  • pytest tests/unit/test_compression_brain_prefetch_logging.py — 2 passed.
  • With engine/compression.py reverted to main and the tests kept, test_failed_brain_prefetch_is_logged fails on AssertionError: expected a 'Brain pre-fetch failed' warning; got: [], whilst the positive control still passes.
  • pytest tests/ -m "not stress" -n 4 against a live SurrealDB v3.2.0 — 7285 passed, 48 skipped, 1 xfailed, which is main's 7283 plus the two 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 #191–fix(memory): refresh content-derived fields on compress, restore, and refine #193.

Related issues

Follow-up 2 from the #193 review. Follow-up 1 from the same review — the GRAPH_ONLY_PLACEHOLDER constant — is a separate PR, since it touches a different set of files for a different reason.

Verified by

@RobertSigmundsson

…rading in silence

CompressionEngine.run fetches the brain once for the whole pass, because the
derived-field refresh needs its embedding config and a per-fiber lookup would
contradict this path's own round-trip budget. When that fetch raised, the
except set brain to None and said nothing — the only unlogged except in a
file where every other one logs.

The fallback itself is right: the refresh helper looks the brain up per fiber
when it is handed None, so the pass completes. Two consequences were invisible
though. The one-lookup-per-pass optimisation degrades to one lookup per fiber
with nothing in the log to explain why the pass got slower. And when the
storage problem persists, the warning the operator does see comes from
content_refresh and blames the embedding provider for what is really a
brain-fetch failure.

A logger.warning with exc_info before the fallback fixes both, and changes
nothing about when the code runs.

Two tests: a pass whose get_brain raises must log the warning and still
return a report, and a pass that fetches its brain must stay quiet.

@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. Fail-soft stays fail-soft, but the degraded brain pre-fetch now warns with the cause instead of blaming the embedder downstream. Green CI.

@acidkill
acidkill merged commit b563322 into acidkill:main Sep 7, 2026
9 checks passed
@acidkill acidkill mentioned this pull request Sep 7, 2026
4 tasks
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