Conversation
Replaces the flat `Media(type, url)` model with a four-variant tagged union — `MediaPhotoPending` / `MediaPhotoDownloaded` / `MediaPhotoFailed` / `MediaVideoPending` — discriminated by `kind`. Mirrors the `ContentSource` design from #20: illegal states (e.g. "downloaded" without `local_path`) are unrepresentable. A `_normalise_legacy_media` BeforeValidator promotes the pre-Phase-A `{type, url}` shape on read so existing `data/items.json` files load without a manual migration. A `Media(...)` factory function preserves the constructor signature used by `extract/graphql.py` and `archive.py` — those modules are out of scope for #33 and keep working unchanged. `MediaFailureReason` adds the categorised failure bucket (http_4xx / http_5xx / timeout / format_error / unknown_error) the downloader will fill in. Transient vs permanent semantics mirror #19. Tests cover legacy migration (photo + video), pass-through of the new shape, the discriminator rejecting unknown kinds, required-field enforcement on the failure variant, the factory routing, and a full `Item` round-trip through the legacy media shape. Refs #33
`xbrain.media.download_all` walks every photo entry across the store, downloads bytes from pbs.twimg.com with a size cascade (`name=orig` → `name=large` → `name=medium`), validates the bytes with Pillow, and writes the file atomically (tmp + rename) under `data/media/<item-id>/<index>.<ext>`. Failure handling mirrors `xbrain.fetch` (#19/#20): - 4xx → `http_4xx` (permanent) - 5xx → `http_5xx` (transient — auto-retried next run) - `requests.Timeout` → `timeout` (transient) - Connection errors / unknown exceptions → `unknown_error` (transient) - Bytes that Pillow can't decode → `format_error` (permanent) `KeyboardInterrupt` propagates untouched (narrow except clauses). A total failure (all attempts failed) raises `RuntimeError`, mirroring `ApiExecutor.enrich_items` (#24) — the CLI's `_handle_cli_errors` turns this into a clean operator message + exit 1. Idempotency: a re-run is a no-op for `MediaPhotoDownloaded`. A progress callback fires after each photo transition, so the CLI can persist `items.json` between photos — Ctrl-C mid-batch leaves a coherent store. I/O is dependency-injected (`session`, `sleep`, `on_progress`) so tests run offline with a hand-rolled `FakeSession` — no `requests-mock` dependency in the unit path. Tests cover: 200-on-orig success, cascade fallback, idempotency, all five failure buckets, `--force`, `--limit`, `--items` filter, throttling, progress callback, video-pending no-op, Ctrl-C propagation, total-failure raise, partial-failure no-raise, atomic write (no `.part` residue), summary-line emission silence vs. full counters. Adds Pillow as a runtime dep (`uv add pillow`). Refs #33
The `media` command wires the Phase A downloader into the CLI: 1. Auto-snapshots `data/` first via `_auto_snapshot(cfg, "media")`, reusing the #17 destructive-op recovery boundary. 2. Loads `items.json`, runs `xbrain.media.download_all` with a per-photo `on_progress` callback that persists the store atomically — Ctrl-C mid-run leaves a coherent `items.json`. 3. Persists once more in a finally block so the partial state from a `RuntimeError`-on-total-failure is not lost. 4. Emits the SUMMARY line on stderr (mirror of #24) and an operator-friendly recap on stdout. Flags: `--force` (re-download already-downloaded photos), `--limit N` (cap attempts), `--items <a,b,c>` (filter by id). `Config.media_dir` resolves to `data/media/` — under the existing gitignored `data/*` umbrella, no `.gitignore` change needed. Tests cover the snapshot-before-run invariant (mirrors the existing fetch/vocab/topics auto-snapshot tests), the full happy path through a fake `requests.Session` (no real network), the `--items` filter, and the total-failure → exit-1 contract with the partial-state-persisted invariant verified. Refs #33
`xbrain generate` now handles every media variant introduced by #33: - `MediaPhotoDownloaded` → embedded inline as `![[_media/<id>/<n>.<ext>]]` after the tweet body, before the `## Enlaces` section. Files are mirrored from `data/media/` into `<output_dir>/_media/` at render time so the vault is fully self-contained — no Obsidian config or symlinks needed on the user side. - `MediaPhotoFailed` → one-line ⚠ warning carrying the original URL and the failure reason (translated to user-facing Spanish via `_FAILURE_ES_MEDIA`). - `MediaPhotoPending` → silent. Not an error, just "not yet processed". - `MediaVideoPending` → 🎥 placeholder with the URL. Phase A does not download videos; the URL is the only evidence we surface. `_has_note` extended: an item with only media (no link, no enrichment) is now note-worthy. A photo-only tweet was previously invisible in the wiki — now it gets its own page as soon as `xbrain media` advances the variant. Render order in the Tweet section: tweet text → photos → links → external article content Deviation from the implementation plan: the plan called for embeds pointing at `data/media/<id>/<n>.<ext>` and "verifying in the end-to-end test" that Obsidian could resolve the relative path. That only works when the vault root and `data/` align, which is not the common case. Mirroring the photos into `<output_dir>/_media/` makes the embed resolve unconditionally for ~zero extra disk (the canonical copy stays at `data/media/`; the vault copy is a render-time artifact). Documented in README. Tests cover all four render branches, the media-block-before-Enlaces ordering, multi-photo inline rendering, missing-bytes recovery (logs + keeps embed), and backward-compat when `generate(media_root=None)`. Refs #33
`DiffReport` now carries a `media: MediaDiff` block with per-variant counts on both sides (A/B) plus the four deltas (`delta_downloaded`, `delta_pending`, `delta_failed`, `delta_video_pending`). `DiffSummary` surfaces the two highest-signal counters (`media_delta_downloaded`, `media_delta_failed`) at the top level for quick-glance ops review. The text formatter adds a `MEDIA` block with right-aligned A/B columns and `±N` deltas. The JSON formatter is automatic via pydantic — `media` becomes a fifth top-level key. The CLI tests that asserted the top-level key set have been updated. Negative deltas are legal — `xbrain media --force` can move a previously-downloaded photo back to `failed` if the URL turned 404 in the meantime. `MediaStateCounts` carries the absolute counts unsigned; the deltas carry sign. Refs #33
README: - Add `media` row to the Commands table. - New "Local media storage" section covering on-disk layout (`data/media/<id>/<n>.<ext>`), vault rendering convention (mirrored into `<output_subdir>/_media/` for self-contained embeds), disk budget guidance, throttling, and the transient/permanent failure-retry contract. ARCHITECTURE: - New "media" entry under per-stage detail (state machine, Ctrl-C safety, snapshot trigger). - Artifacts table row for `media/<id>/<n>.<ext>`. - Invariant #10: media variants are mutually exclusive states; state transitions only via `xbrain media`; legacy `{type, url}` records migrate on read. Refs #33
No behaviour change. ruff format normalises trailing commas, line breaks and trivia in the files touched by the Phase A series. Refs #33
Split the 19-line orchestrator into three small helpers: - `_build_session` extracts the new-vs-injected session decision so `download_all` does not carry a one-off branch. - `_eligible_items` is a generator that filters by `items_filter` and skips items without media, so the orchestrator reads as a flat "for each eligible item, process it" loop. - `_DownloadContext` is a frozen dataclass that bundles the inner- loop deps (session, timeouts, throttle, callbacks, force) so `_process_item` takes one context argument instead of seven. `download_all` drops from C(19) → B(7); `_process_item` stays at B(10); no behaviour change, all 412 tests still pass. Refs #33
`xbrain.media` imports `requests` directly for the HTTP session. Pulling it through trafilatura's transitive dep was a deptry DEP003 warning; declaring it explicitly is the right hygiene. No behaviour change — the same `requests-2.34.2` was already installed via trafilatura. Refs #33
… wrap write - Narrow `_decode_image` exception set to Pillow-specific (UnidentifiedImageError, DecompressionBombError, SyntaxError). Parent OSError swallowed real I/O bugs (disk/permission/network). - Wire `logger.warning` per failure in `_record_outcome` — the total-failure RuntimeError now actually has the breadcrumbs it references. - Wrap `_write_bytes` call in `_download_one` with `try/except OSError`, bucketing disk-full / permission / read-only-fs errors as `unknown_error` (transient) so a single bad write doesn't abort the whole batch. - Sweep stale `*.part` files at the entry to `download_all` to clean up SIGKILL/OOM orphans from previous runs. - Document persistence-failure semantics in `_run_media` docstring. - Add `AVISO:` stderr line on `--items` filter that matches no item. - Add `--verbose` flag printing failed (item_id, reason, url) at the end.
- Drop SessionProtocol (defined for one use); use `requests.Session | None` directly in `download_all`. - Drop `_DownloadContext` dataclass, `_process_item` helper, and the mutable-list `remaining=[limit]` "box" trick. Inline the per-item loop into `download_all` with a plain `int | None` counter and `enumerate(item.media)` instead of `range(len)`. - Inline `_build_session` (used once), `_eligible_items` (used once), and `_finalise` (one-line, called twice) into `download_all`. - Drop `original` param from `_record_outcome` and `entry` param from `_failed` — both were unused and rationalised by speculative comments. Rewrite the now-truthful docstrings. - Move `_REASON_SEVERITY` to module top alongside `_TRANSIENT_MEDIA_FAILURES` and `_SIZE_CASCADE` (was defined mid-file). Drop the unused `format_error: 5` row — `format_error` is an early-return path, never cascade-compared. - Cap `_format_error` output at 500 chars to keep hostile CDN bodies out of `items.json`. - Add `typing.assert_never` exhaustiveness at the three media-variant isinstance chains (media.py:_is_eligible, generate.py:_render_media_lines, diff.py:_count_media_variants) so Phase B catches missed variants statically. - Trim media.py module docstring from 5 paragraphs to 2.
- `MediaPhotoDownloaded.width / height / bytes_size` now `Field(gt=0)`.
A zero-pixel or zero-byte downloaded photo is semantically illegal;
Pillow validation rules it out at the seam, the type pins it at the
data layer.
- `MediaPhotoDownloaded.local_path` gets a pydantic validator that
rejects absolute paths and `..` components. The downloader never
builds such a path, but `items.json` is on-disk plain JSON the user
can edit — defence in depth at the type boundary is cheap.
- `MediaPhotoFailed.attempts` drops its default and becomes
`Field(ge=1)`. "Failed but never attempted" is nonsense; the
downloader always increments before producing the variant.
- `_MediaPhotoBase` docstring corrected: `type` was never the
discriminator, it's preserved for wire-compat with the legacy
`{type, url}` shape.
- `Media()` factory annotated `# noqa: N802` (the rule is intentional)
with a TODO marker to remove when the extractor/archive migrate to
direct variant construction.
- Tests updated to pass `attempts=1` where they previously relied on
the default.
- Strip `Phase A`, `Phase B`, `(#33)`, `pre-#33`, `pre-Phase-A` markers from new/modified code, docstrings, and tests. Also strip `(#17)`, `(#19)`, `(#20)`, `(#24)` references added in this PR's diff. The PR description carries the issue link; code should describe lasting invariants, not the PR that introduced them. - Rewrite `_TRANSIENT_MEDIA_FAILURES` cross-reference comment: was `_TRANSIENT_FAILURES` (the actual symbol in fetch.py). - Reword `Failed(transient)` "terminal-ish" phrasing in ARCHITECTURE.md `### media` — `Failed(transient)` IS auto-retried; not terminal. - Reconcile snapshot/media docs: snapshots cover only the four JSON artifacts; photo bytes under `data/media/` are NOT snapshotted today. Updated `config.py:media_dir` docstring and ARCHITECTURE.md to make the carve-out explicit; re-downloading is the recovery path. - Trim ARCHITECTURE.md invariant #10 to match the brief style of invariants #1-#9. Retry contract and storage layout moved to the `### media` section above (now with storage layout subsection). - Trim the `media` row in README.md Commands table to one sentence with a link to `Local media storage`. Mark the disk-budget numbers as approximate.
…bose - Split the old `test_download_all_records_http_4xx_failure_when_cascade_exhausted` into two: one asserts the bucket via a partial-success setup; the other asserts the lone-attempt RuntimeError. Mixing both concerns made the original test confusing. - Add `test_download_all_falls_back_through_full_cascade_to_medium`: orig 404 + large 404 + medium 200 — proves the third cascade step works. - Add WebP and JPEG extension tests: `.webp` and `.jpg` URLs end up with the right extension on disk. - Add 3xx (304) handling: bucketed as `unknown_error` (transient). - Add atomic-write rollback test: simulate `Path.replace` raising; the `.part` file is cleaned up and no half-written final file remains. - Add `_sweep_part_orphans` smoke test: a stale `*.part` left by a SIGKILL is removed on next `download_all` entry. - Add partial-failure SUMMARY line test: a run with 0 downloads + N failures is NOT silent — ops needs the count. - Add Pillow truncated-PNG test: `_png_bytes()[:32]` lands in the `format_error` bucket. - Add `test_diff_media_reports_delta_failed`: diff surfaces a `delta_failed` count between snapshots (both on `media.delta_failed` and `summary.media_delta_failed`). - Add `test_media_command_resume_after_interrupt_completes_remaining`: the full Ctrl-C-and-resume integration round. - Add `test_media_command_warns_when_items_filter_matches_nothing`: AVISO line on a no-match `--items` filter. - Add `test_media_command_verbose_lists_failed_urls`: `--verbose` prints `<item_id> <reason> <url>` per failure on stderr. - Tighten `test_media_command_runs_on_empty_store`: assert exit-code 0 AND items.json is byte-identical (no spurious rewrite). - Fix `test_generate_renders_failed_photo_as_warning`: replace `"URL no encontrada" in body or "HTTP 4xx" in body` with the actual rendered string `"URL no encontrada (HTTP 4xx)"`.
- Add `_ = cls` line to `_reject_path_traversal` to silence vulture's unused-variable warning; pydantic's `@field_validator + @classmethod` contract requires the `cls` parameter even when the body does not reference it. - Apply `ruff format` to `tests/test_cli.py`. No semantic changes.
The placate-vulture stub (`_ = cls`) was the first statement of the function body, so Python parsed the subsequent triple-quoted block as a no-op expression rather than as the function's docstring. Result: inspect.getdoc(MediaPhotoDownloaded._reject_path_traversal) returned None, hiding the rationale for the validator from tooling and IDEs. Move the docstring to the first-statement position (where Python looks for it) and put the vulture stub immediately after.
The new MediaPhotoDownloaded / MediaPhotoFailed validators (path traversal, gt=0 dimensions, ge=1 attempts) were only exercised by the happy paths. Add explicit rejection tests so a future refactor that weakens a constraint trips a red test, not a silent regression at the data boundary. Six new tests, one per rejection path: - absolute local_path - parent-traversal local_path - bytes_size=0 - width=0 - height=0 - attempts=0 on MediaPhotoFailed
[#33] Phase A: download X-post photos and render in notes
After round-1's inline-the-helpers fix, download_all hit radon C(18) and _download_one C(11) — they carried inherent orchestration complexity that flat inlining could not shed. Targeted refactor without re-introducing single-caller-rename helpers: - Extract _iter_eligible_attempts as a generator yielding the (item_id, item, index, entry) work tuples. The generator carries the filter + empty-media skip + eligibility cascade + global limit countdown — real state-machine logic, not a renamed parameter list. download_all becomes a flat loop with one decision per iteration. - Extract _filter_by_ids for the items-filter restriction (was inlined inside the generator; pulling it out shaves the and-conjunction from the inner loop). - Extract _classify_status to fold the 4xx / 5xx / other branches in _download_one into a single lookup-then-bucket line. Result: download_all C(18) -> B(7) _download_one C(11) -> B(9) _iter_eligible_attempts (new) B(10) No grade C functions remain in media.py. All 431 tests pass; ruff lint + format + mypy clean; behaviour unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
[#33] reduce download_all + _download_one complexity below grade C
Introduce a fifth media variant carrying the vision-LLM output: is_decorative + description + description_lang + description_version + described_at, on top of the downloaded-state fields. The discriminated union grows from four to five `kind`s; the path-traversal validator carries over verbatim, since the on-disk bytes are inherited from the prior downloaded state. Update every isinstance cascade that walked the union (media.py eligibility, generate.py rendering + mirror, diff.py counting) to handle the new variant explicitly so `assert_never` keeps the exhaustiveness check honest. The diff report grows a `described` column and a `delta_described` field; the count is exposed through both the text and JSON renderers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The vision-describe call ships a declarative system prompt — same
shape as the other rubrics. It enumerates the decorative-vs-content
classification, the per-image description contract (1-3 sentences,
plain prose, faithful), the refusal fallback (decorative + empty
description), and the exact JSON output shape (list of {index,
is_decorative, description}).
The `{language}` placeholder follows the existing rubric convention
so `output_language` propagates into vision calls the same way it
already propagates into summary/topic-page rubrics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The orchestrator walks every eligible photo, batches the bytes into vision-API calls (default 5 images per call), parses the per-image JSON list judgments, and transitions matched entries to MediaPhotoDescribed. Decorative classifications are persisted with an empty description so downstream consumers (enrich-time prompt, topic-synth prompt) can filter them out at a single seam. The shape mirrors xbrain.executors.api and xbrain.topic_synth: narrow recoverable-errors tuple, per-batch failure isolation, logger.warning on every failure, RuntimeError on total failure, and a SUMMARY: described/failed/skipped stderr line. Programmer bugs (AttributeError) and KeyboardInterrupt propagate as their narrow catch is on Exception subclasses only. Eligibility uses a description-version tag: bumping the configured version invalidates persisted descriptions automatically (no --force needed). The Ctrl-C-coherent invariant is delegated to an on_progress callback the CLI wires to a store write. 41 unit tests exercise idempotency, force/stale-version, batching, partial+total failure, refusal handling, language plumbing, programmer-bug propagation, KeyboardInterrupt, file-missing fallback, summary line, and clock injection — all via FakeAnthropic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CLI surface mirrors xbrain media: --force, --limit, --items, --model, --batch-size, --verbose. The output_language and the description_version both come from config (config.toml + new [describe] section); --model overrides config.describe_model per invocation. The pre-describe snapshot is auto-taken so a runaway prompt or wrong model can be rolled back with xbrain snapshot restore. Persistence is double-anchored: on_progress writes the store between batches (Ctrl-C-coherent invariant) and a finally clause catches any total-failure RuntimeError so the per-photo MediaPhotoDescribed records that landed before the raise are not discarded. config.toml.example documents the new [describe] section with defaults (Sonnet 4.6 + version v1). 4 CLI tests cover the happy path, no-op exit-0, --items mismatch warning, and total-failure exit-1 propagation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The enrich-time prompt in executors/api.py now carries an "Images in this post:" section listing every non-decorative MediaPhotoDescribed entry's prose, sitting between the post body and the links/article block so the LLM reads context in natural order. Decorative photos are filtered at the seam — they introduce no topic noise. TopicInput grows an image_descriptions field; build_topic_inputs flattens content-bearing descriptions across every post in the topic and feeds them to the topic-synth prompt below the per-post summaries. Same decorative-filter as enrich. The prompt shape stays back-compat: items / topics without described photos emit no images section at all. 8 new tests guard the integration: inclusion, decorative-filter, omission-when-empty, ordering vs links/article. Both _user_prompt helpers were split into per-section builders to keep complexity under grade C — no new C functions introduced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The diff module already grew the described column + delta_described field in the model commit. This commit adds the two regression tests that prove the diff surfaces a downloaded → described transition between snapshots and that the text renderer emits the new "described:" row alongside the existing four. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
README grows a Commands row for describe, a Configuration row for [describe].model and [describe].version, and a Vision descriptions subsection in the Local media storage section that explains the $3-5 budget, the decorative-filter, and the description-version re-describe trigger. ARCHITECTURE adds a Per-stage describe section between fetch and vocab covering the state machine, batching, refusal handling, failure isolation, snapshot trigger, and the consumption seam in executors/api.py + topic_synth.py. The rubrics table grows a describe-image row; invariant #10 names the fifth Media variant and makes the linear-pipeline shape explicit (Pending → Downloaded → Described, with Failed as the off-ramp from Pending). Also bundles two formatter-only cleanups picked up by ruff format on the executors/api.py and topics.py changes from the prompt integration commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop `_print_partial_failure_summary` from `describe_all`: the CLI's
`describe_emit_summary_line` is the single source of truth (mirrors
`media`). Eliminates the double `SUMMARY:` emission on partial-failure
runs (python-code-reviewer BLOCKER).
- `MediaPhotoDescribed` now carries:
- `description_lang: SupportedLanguage` (typed alias over
`i18n.SUPPORTED_LANGUAGES`) — rejects unknown languages at the
type boundary (type-design-analyzer).
- Model validator enforcing `is_decorative => description == ""`
so a hand-constructed variant cannot violate the invariant
(type-design-analyzer).
- `described_at` UTC-aware enforcement via field_validator.
- Dedupe the `local_path` traversal guard via a shared free function
(`_reject_local_path_traversal`); same for the UTC-aware check
(`_require_utc_aware`). Closes the validator duplication called out
by python-code-reviewer + code-simplifier — done via free functions
rather than class inheritance because Liskov substitution would
silently re-match `MediaPhotoDescribed` against 25+
`isinstance(entry, MediaPhotoDownloaded)` call sites.
- Add the same UTC-aware contract to `MediaPhotoDownloaded.downloaded_at`
and `MediaPhotoFailed.last_attempt_at` (closes the pre-existing gap
for the variants this PR touches).
- Tests: orchestrator no longer emits SUMMARY on partial failure
(assert `err.count("SUMMARY:") == 0` — pinning the dedup). Test
helpers for decorative described-photo construction honour the new
empty-description invariant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Convergent fixes from the round-1 reviewer pipeline.
Category 2.1 mixed-language drift (type-design-analyzer + silent-failure-hunter):
- `_is_stale` now also returns True when `description_lang` drifted vs
the configured output language. Switching `output_language` between
Spanish and English no longer leaves stale-language prose persisted —
the next run re-describes those entries.
- `output_language` parameter is typed `SupportedLanguage`, matching
the new `MediaPhotoDescribed.description_lang` field type. The
orchestrator now propagates a typed language end-to-end.
Category 3 over-decomposed orchestrator (code-simplifier):
- Collapsed the four-helper candidate chain (`_tally_skipped` +
`_iter_item_candidates` + `_take_with_limit` + outer
`_iter_candidates` + inner `_all_eligible` closure) into a single
`_iter_eligible_candidates` generator, mirroring `media._iter_eligible_attempts`.
A small `_tally_idempotency_skip` helper keeps the loop body
under radon grade C; `_filtered_items` keeps the outer scan flat.
- Removed `_take_with_limit`, `_chunked`, `_messages_create`, and
`_user_directive` as standalone helpers. `itertools.batched(...)`
replaces `_chunked`; the SDK call is inlined; the directive is a
small dict literal builder local to the block building.
Category 1.3 dead defence-in-depth (python-code-reviewer):
- Dropped the unreachable duplicate-judgment-index guard in
`_apply_batch_judgments`. The parser already rejects duplicate
indices, making the dict-construction guard dead code.
Category 4 silent-failure regressions (silent-failure-hunter):
- 4.1: SDK `stop_reason="refusal"` now transitions every photo in the
batch to `MediaPhotoDescribed(is_decorative=True, description="")`.
Satisfies the rubric contract at the SDK level so refused batches
make progress instead of churning.
- 4.2: `JSONDecodeError` now logs `response.stop_reason` so a
max-tokens truncation is diagnosable from the warning line alone.
- 4.3: Unknown extension in `_media_type` now emits
`logger.warning` before falling back to `image/jpeg`.
- 4.4: A batch where the model omitted some judgments now bumps
`batches_failed += 1` — the batch did not return complete data, so
the operator's "did this API call do its job" view is honest.
Category 5.3 type design (type-design-analyzer):
- Defined `MessagesClient` + `VisionClient` Protocols local to
`describe.py`. Dropped the `# type: ignore[attr-defined]` on
`client.messages.create(...)`.
Category 6 tests (pr-test-analyzer):
- 6.1: New full-payload kwarg shape assertion (model, max_tokens,
system, role, image+text block ordering, base64 source layout).
- 6.2: `test_describe_all_handles_refusal_as_decorative_empty` plus a
dedicated `_FakeRefusalResponse` fake.
- 6.3: `test_parse_batch_response_strips_fence_without_language_tag`.
- 6.4: `--verbose` CLI test exercising the per-failure listing.
- 6.5: New mixed-language eligibility test
(`test_eligible_described_stale_language_is_eligible`) and CLI
partial-failure SUMMARY count test pinning to exactly one.
- Plus refactored `_FakeMessagesList` so pre-built response objects
(refusal, truncation) bypass the JSON-list wrapper.
Category 7 comments + docs:
- 7.1: Fixed misleading `_MEDIA_TYPES` comment (was "three", actual
cardinality is 4 keys mapping to 3 distinct media types).
- 7.2: Rewrote `_run_describe` docstring — coherence is held by the
outer `try/finally`, not by `on_progress`.
- 7.3: Trimmed the `describe.py` module docstring to 2 paragraphs
matching the project voice (was 4 paragraphs, 22 lines).
- 7.4: Reworded ARCHITECTURE.md `### describe` ("a new variant on
the `MediaEntry` union"). Documented the language-staleness
contract in the state-machine doc.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
transcripts) The producer (`export_topic_worksheet`) emits `image_descriptions`, but the claude-code TOPICS *consumers* dropped it, so `topics --resynth --executor claude-code` never surfaced the images — and every item uses the claude-code track. Wire both consumer artifacts, and fold in the symmetric `video_transcripts` gap (already on `TopicInput`, injected by the api topic path) which closes #56's transcript-to-topics half. Consumers: - `.claude/workflows/resynth-topic-overviews.js`: the per-topic extraction command now prints `image_descriptions` + `video_transcripts` (the agent is told to use only what the command prints), and PASO 2/3 gain "Images across…" and "Video transcripts across…" evidence blocks mirroring `topic_synth._user_prompt`. - `.claude/skills/enriching-x-knowledge/SKILL.md`: the topic-object field list now names `image_descriptions` + `video_transcripts`, and step 3 instructs the session to weigh them as visual/transcript evidence. Producer: - `topic_synth.export_topic_worksheet` also emits `video_transcripts` (per-video-bounded by `build_topic_inputs`, empty list when none). Guard + tests (TDD; producer tests can't catch a consumer that drops a field): - `tests/test_worksheet_consumer_wiring.py`: text-level assertions that the workflow extraction/prompt and the skill field list carry BOTH new fields — fails if a future edit removes them from the consumer. - Producer tests for `video_transcripts` (present + empty), mirroring the image tests. Minor: - Worksheet + topic-worksheet `instructions` now name the extra evidence (links / article / transcript / image descriptions) as signal to weigh. - Docs (ARCHITECTURE / CLAUDE / README): replace the vague "forced re-run" with the actual propagation commands — `xbrain vocab --regenerate` (clears enrichments) then `enrich`, and `xbrain topics --resynth`; note the workflow + skill consumers surface the field. - Test-quality comment on the decorative worksheet test: the model invariant `is_decorative => description == ""` makes the filter's decorative clause independently untestable (the empty-description backstop already excludes it). Not in scope: frame-descriptions -> topics (remaining #56 piece; frames aren't on TopicInput), the api path, and the #44 video code beyond the one export line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t enrich/topics tracks (#62) Closes #34; closes the transcript-to-topics half of #56. Producer + consumer wiring so content-bearing image descriptions (and bounded video transcripts) reach the LLM on the claude-code worksheet track, not just the API track. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an additive, ordered `blocks: list[ArticleBlock]` field to `ContentSourceSuccess`, mirroring exactly how `frames`/`has_speech`/ `language` were added for `x_video` in #44 — no new top-level model, no parallel store. `ArticleBlock` is a `kind`-discriminated union: - `ArticleTextBlock(kind="text", text)` - `ArticleImageBlock(kind="image", media: MediaEntry, alt=None)` The image block WRAPS the existing `MediaEntry` photo-state union, so the photo download engine, the `_reject_local_path_traversal` / `_require_utc_aware` validators, the `_media/` mirror and a future describe path all apply with zero new plumbing. `text` stays the source of truth for the flattened body (= concatenation of the text blocks), so enrich/topics/generate's fallback consume it unchanged. Purely additive + back-compat: legacy `items.json` (no `blocks` key, pre-#20 `ok:`/legacy-media shapes) loads unchanged, defaulting `blocks` to `[]`. Model-only PR: no extract/fetch/media/generate behaviour change. Adds `ArticleBlockAdapter` (out-of-`Item` validation, mirrors `MediaEntryAdapter`). Docs: ARCHITECTURE data-model invariant #12 + the `x_article` note; CLAUDE.md model-seam bullet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mageBlock) Additive `blocks: list[ArticleBlock]` on ContentSourceSuccess; the image block wraps the existing MediaEntry photo-state union so the download engine + validators + _media/ mirror apply with no new plumbing. `text` stays the flattened body (unchanged), so enrich/topics/generate's fallback consume it as before. Purely additive + back-compat (proven lossless on 29k real records) — legacy items.json loads with blocks=[]. Model-only; the producer (fetch), download walk (media) and blogpost renderer (generate) land in PR2-5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`digest-video --frames` was locked to whatever single model `[vision].command` pointed at. Add a selector so one command serves local AND cloud vision, picked per run: - scripts/xbrain-vision — a multi-backend wrapper fulfilling the xbrain vision contract (`<cmd> [--model NAME] <image>` → stdout). NAME routes by registry: local (mlx-vlm: qwen-3b/7b/32b or any hf/repo) or cloud (Claude opus/sonnet/ haiku or any claude-<id>). Cloud uses the Anthropic REST API via stdlib urllib — no SDK dependency; local shells to the mlx-vlm tool python. - digest-video --vision-model NAME — overrides [vision].model for the run, so a slide-heavy talk can go to `opus` while the bulk stays local `qwen-3b`. - Docs: README "Local models for digest-video" (install + selector table + per-run examples), config.toml.example [vision] selector block. Tests: --vision-model reaches the vision command; no override falls back to [vision].model. mypy clean, digest+cli suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ards Address PR2 review round (all non-blocking): - dedup the synthesized article link against a canonical URL key (host aliases → x.com, drop scheme + trailing slash) so a twitter.com / http / trailing-slash variant in entities.urls no longer double-adds; - only synthesize for an Article __typename (reject a Card carrying a rest_id); - require a numeric-string rest_id (kills the non-scalar garbage-URL vector); - keep _extract_links at radon A by hoisting the append into a helper; - note the card/unified_card variant as a conscious deferral folded into the same real-payload validation step (docstring + ARCHITECTURE). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract detects the X long-form Article entity on a tweet result (article.article_results.result.rest_id, anchored via _dig) and synthesizes its canonical https://x.com/i/article/<id> Link, routing through the existing rendered-fetch path with no fetch_x change. Normalized dedup against entities.urls (host aliases → x.com, scheme + trailing slash insensitive) plus __typename ("Article"-only) and numeric-string rest_id guards prevent duplicate or garbage links. The Article GraphQL fixture is constructed, not a recorded live payload — validate the key path (and the deferred card/unified_card variant) against a real bookmarked-Article response before production reliance (RFC #39 open-Q #4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- xbrain-vision: guard image read (OSError → clean error), catch URLError (not just HTTPError) on the cloud path, add an inner mlx subprocess timeout (240s < the 300s runner cap, so no orphaned child), sentinel-wrap local output so library stdout noise can't leak into the caption, and unpack tuple/list generate() returns defensively. `haiku` alias → plain `claude-haiku-4-5`. - cli: `--vision-model` without `--frames` is now a hard error (was a silent no-op), matching the flag's documented requirement. - docs: fix README (XBRAIN_VISION_MLX_PYTHON is the LOCAL knob, not cloud) + note large local models must be pre-pulled (300s timeout); drop the stale `qwen2-vl-7b` example the wrapper would reject from config.toml.example. - tests: new tests/test_xbrain_vision.py locks _resolve routing (aliases, claude passthrough, hf/repo, unknown→exit, default) + empty-output→exit-1; plus a CLI test for the --vision-model-without-frames error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-model guard CI (80-col) wrapped/styled the typer BadParameter panel, breaking the literal '--frames' substring match that passed at a wider local width. Assert exit_code == 2 (click usage error) + strip ANSI before matching. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat(digest): multi-backend vision model selector + docs (#56)
…ant) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ot-crash, determinism pin - article.py: WARN on a dropped entity-bearing block (real-payload key-drift visibility); `_find_url_by_key` now prefers the canonical CDN key globally (deep media_url_https beats a shallow bare url); decompose _coerce_content_state. - fetch_x.py: INFO structured/fallback, WARN on captured-but-0-blocks (silent-death signal) + on structured-body truncation vs trafilatura (<50%); DEBUG the .json() swallow; wrap parse in try/except → degrade to fallback on ANY exception (incl. RecursionError); `_attach_x_sources` gains an injectable `now=` clock. - models.py: document that an image-only article legitimately has text=="". - Determinism: pin PYTHONHASHSEED in scripts/check.sh + quality.yml; mock trafilatura in the structured-path test. - Tests: decoy-blocks-key gate, canonical-URL preference, dropped-block warning, captured-but-0-blocks warning, truncation warning, parser-exception fallback, injectable-clock. README: "reads like a blogpost" → "can later render as one". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rade, not crash) The truncation diagnostic runs on the structured SUCCESS path; a trafilatura failure there must not discard the already-built good structured body. Wrap the trafilatura.extract call in try/except → skip the check (DEBUG), keep the body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lidator, observability, tripwire) Structured X-article capture: intercept the article-content GraphQL response and parse its Draft.js content_state into an ordered ArticleBlock body (text runs + MediaPhotoPending inline images, in document order); degrade to the retained trafilatura.extract text-only fallback on any interception/parse miss (never a crash, never a partial body masquerading as complete). The flattened `text` is enforced to equal the concatenation of the text runs by a ContentSourceSuccess model_validator. x_article re-enrich hygiene: _attach_x_sources bumps fetched_at only on a material change (reused fingerprint). Observability + a truncation tripwire surface silent degradation; the parser degrades on any exception. Fixture + article GraphQL op-name are CONSTRUCTED (no real captured payload) — validate against a real bookmarked-Article response before production reliance (RFC #39 open-Q #4). PR3 attaches blocks with pending images; the image DOWNLOAD is PR4 and the blogpost RENDER is PR5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nerator, docs scope-down Fix round addressing PR4 reviews: - Dedicated MediaReport.article_images_skipped counter (stop folding article skips into photos_skipped_already_downloaded); surfaced in SUMMARY + CLI echo. - Thread --limit into _iter_eligible_article_images (top-of-iteration budget check, matching the photo generator) so a spent budget no longer scans past and miscounts skips. - Docs: scope down two render/mirror overclaims (article render/mirror is PR5, not PR4) in README + ARCHITECTURE; reconcile skip-counter wording in CLAUDE. - Tests: article-only total-failure raises; two-x_article-source global index continuation; limit-exhaustion skip-count parity; dedicated skip-counter assertions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…57) Diagnosis: a large share of "transcription failed" videos are simply SILENT — no audio track at the source (muted clips, GIFs, screencasts; confirmed with yt-dlp that no audio format exists, ruling out separate audio streams). parakeet -mlx writes no output file for them, which xbrain's transcribe contract treats as a hard failure — even though xbrain's own design wants them attached as has_speech=false ("silent video" note). - scripts/xbrain-transcribe: wraps parakeet-mlx; when parakeet exits 0 but wrote no file, `ffprobe`-checks the media — no audio stream → emit the empty-speech JSON (silent video); HAS audio but no output → surface as a real failure (not masked as silent). Non-zero parakeet exit propagates. Local, no API. - docs: README "Transcriber wrapper" note + config.toml.example [transcribe]. - tests/test_xbrain_transcribe.py: locks the 4 branches (speech pass-through, silent→empty-speech, audio-but-empty→fail, parakeet-nonzero→propagate) + the no-ffprobe fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nscribe wrapper (PR #64 review) Review (python-bug-finder) surfaced that xbrain reads the first NON-EMPTY *.json by content (transcribe.py glob), not <stem>.json by name. Hardened accordingly: - Detect "parakeet produced a transcript" as any non-empty *.json (mirrors xbrain), not `<stem>.json`.exists(). Fixes: an empty/stub file no longer fools the wrapper into skipping silent-classification (finding #1), and output under an unexpected filename is still detected (removes stem coupling, finding #4). - `_confirmed_no_audio`: classify silent ONLY when ffprobe positively confirms no audio (returncode 0 + no audio stream). Unknown (no ffprobe / probe error) → real failure, never masked as silent (finding #3). - `XBRAIN_PARAKEET` is shlex-split so a multi-token wrapper command works. - Tests: empty-stub-not-output, non-stem output name, confirm-silence truth table (no-audio / audio / probe-error / no-ffprobe), + the four main() branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(digest): handle silent (no-audio) videos instead of failing them (#57)
… branch Address the PR5 review panel (all non-blocking): - Skip the bare `## Content:` heading when an x_article's blocks render to nothing (e.g. an all-pending image-only Article), mirroring `_video_digest_lines`' empty-block guard. - Add a test for the video-variant-on-article-image defensive branch (logged + skipped, no embed, no crash). - Strengthen the pending-image-silent test (no `![[` embed, no dead URL leaks). - Tighten `_article_caption_lines` type hint to `MediaPhotoDownloaded | MediaPhotoDescribed`. - Fix ARCHITECTURE wording: the failed-image line is a `>` blockquote, and note the empty-body heading suppression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stics Post-review tightening on the #39 chain (all reviewers cleared; small, scoped): - architect M1: hoist the "\n\n" inter-paragraph separator to a single source of truth `models.ARTICLE_PARAGRAPH_SEP`, imported by both the producer (extract.article) and the consumer (generate). The two independent constants coupled only by a comment could drift; now they can't. models is the shared home (both already depend on it; no new generate->extract edge). - data-safety #1: the navigation `except` in fetch_x `_fetch_tweet` / `_fetch_rendered` bucketed every failure as `timeout` and discarded `str(exc)`. Keep the `timeout` classification but capture a capped `str(exc)` into `error` (new `_nav_error`, capped at `_MAX_ERROR_LEN`, mirroring media._format_error) and add `logger.warning(..., exc_info=True)` so a real nav failure on the unvalidated article path is debuggable. - data-safety #4: `_fetch_tweet`'s `on_response` swallowed a non-JSON body with a bare `pass`; now a DEBUG log with the url, matching `_fetch_rendered`. - quality-gates: add the `_needs_x_fetch` `until`-upper-bound test (created_at > until => not fetched), closing the one untested pure-logic branch. No failure_reason reclassification, no render/model/media behavior change. Gate green in-worktree: 1032 tests, 95% coverage, radon A/B. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
[#39] Capture X long-form Articles as blogposts
…eet-mlx, mlx-vlm) The digest-video external tools were only described in the "Local models" section, not surfaced as prerequisites, and ffmpeg (needed for --frames extraction AND the transcribe wrapper's audio probe) was never listed as an install step. - Prerequisites table: add ffmpeg + parakeet-mlx + mlx-vlm (optional — only for digest-video; external, not pulled by `uv pip install`), linking the setup section. - Local-models section: add `brew install ffmpeg` as step 1; document the models that download on first use + their sizes/RAM, and the no-install cloud option. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
docs: surface digest-video external dependencies (ffmpeg, parakeet-mlx, mlx-vlm)
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.
Consolidates ~2 months of
developwork intomain. Every change landed as its own CI-green PR; this is the release cut.Highlights
x_video→ discard bytes.--frames(opt-in) extracts slide key-frames + describes them with an external VLM; talking-heads skipped. Flows through enrich → topics → generate.scripts/xbrain-visionroutes--modelto local mlx-vlm (qwen-3b/7b/32b) or cloud Claude;scripts/xbrain-transcribeclassifies no-audio videos ashas_speech=falseinstead of failing._indexvia afile://link.refresh-media,download-videos.All quality gates green on develop (ruff, mypy, radon, bandit, vulture, interrogate, tests+coverage, deptry).