[#24] Narrow bare except in ApiExecutor + synthesize_overviews_api; doc xbrain diff#31
Merged
Conversation
…f in ARCHITECTURE Closes #24. Two changes in one PR: 1) ARCHITECTURE.md — new "Snapshot diffing" section explaining xbrain diff (#18) end-to-end. The module was listed in "Where things live" but had no narrative. Now covers: what the report contains (Items / Topics / Vocab / Summary), the pure-Python TF cosine choice and rationale (no new deps, IDF degenerates on N=2), the 5-member growth floor, JSON vs text output, and how diff is the foundation for drift monitoring + WS3 eval (#8). 2) ApiExecutor.enrich_items + synthesize_overviews_api — narrow the catch-everything except, count failures, raise on total-failure batches. The pre-fix behaviour: - `except Exception as exc:` swallowed every exception including AttributeError, KeyboardInterrupt, anthropic.AuthenticationError, etc. - A 100%-broken run printed N warn-lines and exited 0. The CLI's downstream commands (xbrain topics, xbrain generate) silently consumed the empty result. The user got a stale wiki without a signal. The fix: - New `_recoverable_errors()` tuple per module: (anthropic.APIError, ValueError, json.JSONDecodeError, KeyError). `pydantic.ValidationError` is a ValueError subclass, covered. Lazy-imported because `anthropic` is optional in some test setups. - Per-item / per-topic failure counter. - `RuntimeError` raised when every input failed (`successes == 0 and failures > 0`) — CLI's _handle_cli_errors catches RuntimeError → clean exit-1. - Partial-failure summary line printed to stderr (`enriched: N, failed: M` / `synthesized: N, failed: M`). - Programmer bugs (AttributeError, ...) and KeyboardInterrupt now propagate — Ctrl-C works again during development, tracebacks surface. Tests (4 new): - `test_api_executor_skips_item_on_api_failure` (updated): uses anthropic.APIError + asserts the partial-failure summary line. - `test_api_executor_raises_when_all_items_fail` (new): every call raises APIError → RuntimeError("All N items failed enrichment"). - `test_api_executor_propagates_programmer_bugs` (new): AttributeError propagates instead of being swallowed. - Same three for synthesize_overviews_api in test_topic_synth.py. Total: 352 tests (up from 348), coverage 89%, `uv run poe check` all-green. PRD: vault/zz-support-files/docs/prds/2026-05-23-xbrain-24-narrow-bare-except.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses the actionable findings from PR #31's 3-reviewer panel (code-reviewer SHIP IT; silent-failure-hunter + test-analyzer flagged improvements): silent-failure-hunter M2 — partial-failure summary line was indistinguishable from the per-item `warn:` lines that precede it. A 1000-item batch with 50 failures buried the single summary at the end of 50 noise lines. - Prefix with `SUMMARY:` so the line stands out. Trivial. - Existing partial-failure asserts updated accordingly. test-analyzer gap #1 (rating 7) — KeyboardInterrupt propagation was implicitly correct (KeyboardInterrupt is a BaseException, not Exception, so the narrow tuple naturally excludes it) but no regression test pinned it. A future refactor that widens the catch to BaseException would silently break Ctrl-C. - New `test_*_propagates_keyboard_interrupt` for both modules. test-analyzer improvement #2 (rating 5) — no negative assert that the all-failed branch does NOT print the summary line (it raises first; a future refactor that moves the print above the raise would slip). - New `test_*_emits_no_summary_on_total_failure` for both modules. test-analyzer improvement #3 (rating 4) — no negative assert that the all-succeed path stays silent on stderr. - New `test_*_emits_no_summary_when_all_succeed` for both modules. Skipped: silent-failure M1 (`except ImportError` opacity for a misinstalled anthropic SDK). The reviewer themselves noted it is "actually safe (loud failure)" since downstream code would propagate the SDK error as a bug rather than catch it silently. Borderline, non-blocker. Total: 358 tests (up from 352), coverage 89%, `uv run poe check` all-green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 23, 2026
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.
Closes #24.
Summary
Two changes in one PR — both small, both follow-ups to the prior review pipelines:
Doc fix (#18 follow-up)
ARCHITECTURE.mdgains a new "Snapshot diffing" section that actually explains whatxbrain diffdoes. Until now the module appeared only in the file-tree at "Where things live"; spec-compliance reviewer on PR #28 flagged the gap. The new section covers:Narrow bare-except (#24 main scope)
ApiExecutor.enrich_itemsandsynthesize_overviews_apihadexcept Exception as exc: print(warn:...); continue. Surfaced by silent-failure-hunter on PR #23 and #29 as a pre-existing issue.Before: every exception including
AttributeError,KeyboardInterrupt,anthropic.AuthenticationErrorwas swallowed. A 100%-broken run printed N warn-lines and exited 0.After:
_recoverable_errors()tuple:(anthropic.APIError, ValueError, json.JSONDecodeError, KeyError). Lazy-imported becauseanthropicis optional in some test setups.RuntimeErrorraised when EVERY input failed (successes == 0 and failures > 0) — CLI's_handle_cli_errorscatchesRuntimeErrorand exits 1 cleanly.enriched: N, failed: M).AttributeError) andKeyboardInterruptpropagate.Tests
4 new in total (2 per module):
test_api_executor_raises_when_all_items_fail— total failure raisesRuntimeError.test_api_executor_propagates_programmer_bugs—AttributeErroris not caught.synthesize_overviews_api.anthropic.APIErrorinstead ofRuntimeError+ assert the new summary line.Total: 352 tests (up from 348), coverage 89%,
uv run poe checkall-green.Specs
vault/zz-support-files/docs/prds/2026-05-23-xbrain-24-narrow-bare-except.mdTest plan
uv run poe checkall-green🤖 Generated with Claude Code