refactor(ingest): per-post handling into run stdout via a downloader outcome (#842)
Two corrections from operator review: 1. Reuse the existing 'Raw stdout' panel instead of a bespoke structured UI section — the native ingester now writes a per-post line into the run stdout (parity with gallery-dl's per-file stdout), so the per-post handling shows in the panel the operator already uses. 2. DRY: stop re-reading post['attributes'] inline in ingest_core. write_post_record now returns a PostRecordOutcome (path, post_type, title, body_chars) — mirroring the download_post -> MediaOutcome contract — and the downloader owns the read; ingest_core only formats the outcome into the log line. Reverts the post_diagnostics metadata field + DownloadDetailModal 'Post capture' section added earlier. Per-post line: 'post <id> [<post_type>] body: N chars' (+ ' — EMPTY' when 0), so an empty body is self-explanatory by post_type. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -477,10 +477,6 @@ class DownloadService:
|
||||
# recurring "post shows a zip but no images" report. Each entry is
|
||||
# {file, reason}; None when every archive extracted cleanly.
|
||||
"unextracted_archives": unextracted_archives or None,
|
||||
# #842: per-post body-capture outcomes ({post_id, title, post_type,
|
||||
# body_chars}) so the UI shows how each post was handled — a 0-char
|
||||
# body next to its post_type explains an empty post at a glance.
|
||||
"post_diagnostics": getattr(dl_result, "post_diagnostics", None) or None,
|
||||
}
|
||||
await self._update_source_health(
|
||||
source_id=ctx["source_id"], status=status, error_message=ev.error,
|
||||
|
||||
@@ -158,11 +158,6 @@ class DownloadResult:
|
||||
# backfilled (inline-image localization) WITHOUT re-download or unlink. Empty
|
||||
# on the gallery-dl path and outside recapture.
|
||||
relink_source_paths: list[tuple[str, str]] = field(default_factory=list)
|
||||
# Native ingester (#842): per-post body-capture diagnostics
|
||||
# ({post_id, title, post_type, body_chars}) surfaced on the DownloadEvent so
|
||||
# the operator can see in the UI how each post was handled. Empty on the
|
||||
# gallery-dl path and for posts whose body wasn't (re)captured this run.
|
||||
post_diagnostics: list[dict] = field(default_factory=list)
|
||||
stdout: str = ""
|
||||
stderr: str = ""
|
||||
return_code: int = 0
|
||||
|
||||
@@ -143,11 +143,6 @@ class Ingester:
|
||||
# media, so phase 3 can backfill the ImageRecord's source_filehash WITHOUT
|
||||
# re-downloading or unlinking the file. Empty outside recapture mode.
|
||||
relink: list[tuple[str, str]] = []
|
||||
# #842: per-post body-capture diagnostics surfaced on the DownloadEvent so
|
||||
# the operator can see IN THE UI how each post was handled — crucially the
|
||||
# post_type + final body length, which explains an empty body (e.g. a poll
|
||||
# whose body the API never returns) without digging through worker logs.
|
||||
post_diag: list[dict] = []
|
||||
downloaded = 0
|
||||
errors = 0
|
||||
quarantined = 0
|
||||
@@ -185,7 +180,6 @@ class Ingester:
|
||||
written_paths=written,
|
||||
post_record_paths=list(post_records),
|
||||
relink_source_paths=list(relink),
|
||||
post_diagnostics=list(post_diag),
|
||||
stdout="\n".join(log_lines),
|
||||
stderr="",
|
||||
return_code=return_code,
|
||||
@@ -266,23 +260,20 @@ class Ingester:
|
||||
else self._seen_keys(source_id, [pkey])
|
||||
)
|
||||
if pkey not in already:
|
||||
rec_path = write_post_record(post, artist_slug)
|
||||
if rec_path is not None:
|
||||
post_records.append(str(rec_path))
|
||||
rec = write_post_record(post, artist_slug)
|
||||
if rec.path is not None:
|
||||
post_records.append(str(rec.path))
|
||||
self._mark_seen(source_id, [(pkey, ppid)])
|
||||
# Read the FINAL body (write_post_record memoized any
|
||||
# detail-fetched content onto the post) + post_type
|
||||
# (already in the feed fieldset) for the per-post UI
|
||||
# diagnostic. A body_chars of 0 with the post_type is
|
||||
# the operator's "why is this one empty" answer.
|
||||
pattrs = post.get("attributes") or {}
|
||||
pbody = pattrs.get("content")
|
||||
post_diag.append({
|
||||
"post_id": ppid,
|
||||
"title": pattrs.get("title") or None,
|
||||
"post_type": pattrs.get("post_type") or None,
|
||||
"body_chars": len(pbody) if isinstance(pbody, str) else 0,
|
||||
})
|
||||
# Per-post handling line in the run stdout (the existing
|
||||
# "Raw stdout" panel) — the downloader already read the
|
||||
# post; we only format its outcome here. post_type beside
|
||||
# a 0-char body is the "why is this one empty" answer.
|
||||
log_lines.append(
|
||||
f" post {ppid} [{rec.post_type or '?'}] "
|
||||
f"body: {rec.body_chars} chars"
|
||||
+ ("" if rec.body_chars else " — EMPTY")
|
||||
+ (f" — {rec.title}" if rec.title else "")
|
||||
)
|
||||
|
||||
media = self.client.extract_media(post, included)
|
||||
if not media:
|
||||
|
||||
@@ -151,6 +151,20 @@ class MediaOutcome:
|
||||
error: str | None
|
||||
|
||||
|
||||
@dataclass
|
||||
class PostRecordOutcome:
|
||||
"""Result of write_post_record — mirrors the download_post → MediaOutcome
|
||||
contract so the engine reports per-post handling without re-reading the post.
|
||||
`path` is the _post.json sidecar (None when the post had no id); the rest is
|
||||
the captured body's shape (post_type + final char count) for the run log.
|
||||
"""
|
||||
|
||||
path: Path | None
|
||||
post_type: str | None
|
||||
title: str | None
|
||||
body_chars: int
|
||||
|
||||
|
||||
class PatreonDownloader:
|
||||
"""Download resolved Patreon media to gallery-dl's on-disk layout.
|
||||
|
||||
@@ -567,27 +581,33 @@ class PatreonDownloader:
|
||||
sidecar_path.write_text(json.dumps(data, indent=2))
|
||||
return sidecar_path
|
||||
|
||||
def write_post_record(self, post: dict, artist_slug: str) -> Path | None:
|
||||
def write_post_record(self, post: dict, artist_slug: str) -> PostRecordOutcome:
|
||||
"""Write a post-ONLY sidecar (no media file) for a media-less post, so
|
||||
the importer can still upsert the Post + its body — text posts often hold
|
||||
the only copy of an external <a href> link. Named `_post.json`: the
|
||||
leading underscore keeps it from colliding with a media sidecar
|
||||
(`<NN>_<stem>.json`) and from being resolved as some media file's sidecar
|
||||
by find_sidecar. Returns the path, or None when the post has no id."""
|
||||
by find_sidecar.
|
||||
|
||||
Returns a PostRecordOutcome (path None when the post has no id) carrying
|
||||
the captured body's shape — post_type + final char count — so the engine
|
||||
can log per-post handling without re-reading the post itself.
|
||||
"""
|
||||
attrs = post.get("attributes") or {}
|
||||
title = attrs.get("title") if isinstance(attrs.get("title"), str) else None
|
||||
post_type = attrs.get("post_type") if isinstance(attrs.get("post_type"), str) else None
|
||||
pid = str(post.get("id") or "")
|
||||
if not pid:
|
||||
return None
|
||||
return PostRecordOutcome(
|
||||
path=None, post_type=post_type, title=title, body_chars=0,
|
||||
)
|
||||
post_dir = self.images_root / artist_slug / "patreon" / _post_dir_name(post)
|
||||
post_dir.mkdir(parents=True, exist_ok=True)
|
||||
path = self._write_sidecar_data(post, post_dir / "_post.json")
|
||||
# Per-post body-capture diagnostic: _write_sidecar_data has, by now,
|
||||
# memoized any detail-fetched body onto post["attributes"]["content"], so
|
||||
# this reflects the FINAL body (feed or detail). Logs the success (N chars)
|
||||
# AND the skip (empty) so a recapture's per-post outcome is visible.
|
||||
# _write_sidecar_data has by now memoized any detail-fetched body onto
|
||||
# post["attributes"]["content"], so re-read it for the FINAL char count.
|
||||
body = (post.get("attributes") or {}).get("content")
|
||||
n = len(body) if isinstance(body, str) else 0
|
||||
if n:
|
||||
log.info("post-record: post %s body captured (%d chars)", pid, n)
|
||||
else:
|
||||
log.info("post-record: post %s has NO body (feed + detail both empty)", pid)
|
||||
return path
|
||||
body_chars = len(body) if isinstance(body, str) else 0
|
||||
return PostRecordOutcome(
|
||||
path=path, post_type=post_type, title=title, body_chars=body_chars,
|
||||
)
|
||||
|
||||
@@ -28,54 +28,6 @@
|
||||
<div><span class="fc-dl-grid__label">Errors</span><span>{{ summary.errors ?? 0 }}</span></div>
|
||||
</div>
|
||||
|
||||
<template v-if="postDiag.length">
|
||||
<h3 class="text-subtitle-2 mt-4">Post capture</h3>
|
||||
<div class="fc-dl-grid">
|
||||
<div><span class="fc-dl-grid__label">Posts</span><span>{{ postDiag.length }}</span></div>
|
||||
<div><span class="fc-dl-grid__label">With body</span><span>{{ postWithBody }}</span></div>
|
||||
<div><span class="fc-dl-grid__label">Empty body</span><span>{{ postEmpty.length }}</span></div>
|
||||
</div>
|
||||
<v-expansion-panels class="mt-2">
|
||||
<v-expansion-panel v-if="postEmpty.length">
|
||||
<v-expansion-panel-title>
|
||||
<span>Empty-body posts ({{ postEmpty.length }})</span>
|
||||
</v-expansion-panel-title>
|
||||
<v-expansion-panel-text>
|
||||
<table class="fc-dl-table">
|
||||
<thead><tr><th>Post</th><th>Type</th><th>Chars</th></tr></thead>
|
||||
<tbody>
|
||||
<tr v-for="p in postEmpty" :key="p.post_id" class="fc-dl-empty">
|
||||
<td>{{ p.title || `#${p.post_id}` }}</td>
|
||||
<td><code>{{ p.post_type || '—' }}</code></td>
|
||||
<td>{{ p.body_chars }}</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
</v-expansion-panel-text>
|
||||
</v-expansion-panel>
|
||||
<v-expansion-panel>
|
||||
<v-expansion-panel-title>
|
||||
<span>All posts ({{ postDiag.length }})</span>
|
||||
</v-expansion-panel-title>
|
||||
<v-expansion-panel-text>
|
||||
<table class="fc-dl-table">
|
||||
<thead><tr><th>Post</th><th>Type</th><th>Chars</th></tr></thead>
|
||||
<tbody>
|
||||
<tr
|
||||
v-for="p in postDiag" :key="p.post_id"
|
||||
:class="{ 'fc-dl-empty': !p.body_chars }"
|
||||
>
|
||||
<td>{{ p.title || `#${p.post_id}` }}</td>
|
||||
<td><code>{{ p.post_type || '—' }}</code></td>
|
||||
<td>{{ p.body_chars }}</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
</v-expansion-panel-text>
|
||||
</v-expansion-panel>
|
||||
</v-expansion-panels>
|
||||
</template>
|
||||
|
||||
<template v-if="quarantinedPaths.length">
|
||||
<h3 class="text-subtitle-2 mt-4">Quarantined paths</h3>
|
||||
<ul class="fc-dl-quar">
|
||||
@@ -163,31 +115,18 @@ const summary = computed(() => props.event?.metadata?.import_summary || {})
|
||||
const quarantinedPaths = computed(() => props.event?.metadata?.quarantined_paths || [])
|
||||
const errorsWarnings = computed(() => props.event?.metadata?.stderr_errors_warnings || '')
|
||||
|
||||
// #842: per-post body-capture diagnostics — how each post was handled, with
|
||||
// post_type so an empty body is self-explanatory in the UI.
|
||||
const postDiag = computed(() => props.event?.metadata?.post_diagnostics || [])
|
||||
const postEmpty = computed(() => postDiag.value.filter((p) => !p.body_chars))
|
||||
const postWithBody = computed(() => postDiag.value.length - postEmpty.value.length)
|
||||
|
||||
// One combined block for "research the issue elsewhere" — header line +
|
||||
// error + full stdout/stderr. Built lazily from the current event.
|
||||
const allDiagnostics = computed(() => {
|
||||
const e = props.event
|
||||
if (!e) return ''
|
||||
const md = e.metadata || {}
|
||||
const emptyPosts = postEmpty.value
|
||||
.map((p) => ` • ${p.title || '#' + p.post_id} [${p.post_type || '—'}] ${p.body_chars} chars`)
|
||||
.join('\n')
|
||||
return [
|
||||
`event #${e.id} · ${e.platform || '—'} · ${e.artist_name || '—'}`,
|
||||
`status: ${e.status}`,
|
||||
`started: ${e.started_at} finished: ${e.finished_at || '(running)'}`,
|
||||
e.error ? `\n--- error ---\n${e.error}` : '',
|
||||
errorsWarnings.value ? `\n--- errors & warnings ---\n${errorsWarnings.value}` : '',
|
||||
postDiag.value.length
|
||||
? `\n--- post capture (${postWithBody.value}/${postDiag.value.length} with body) ---`
|
||||
+ (emptyPosts ? `\nempty-body posts:\n${emptyPosts}` : '')
|
||||
: '',
|
||||
`\n--- stdout ---\n${md.stdout || '(empty)'}`,
|
||||
`\n--- stderr ---\n${md.stderr || '(empty)'}`,
|
||||
].filter(Boolean).join('\n')
|
||||
@@ -242,25 +181,6 @@ function onClose() {
|
||||
word-break: break-all;
|
||||
}
|
||||
.fc-dl-quar { padding-left: 1.5rem; font-size: 0.85rem; }
|
||||
.fc-dl-table {
|
||||
width: 100%;
|
||||
border-collapse: collapse;
|
||||
font-size: 0.8rem;
|
||||
}
|
||||
.fc-dl-table th,
|
||||
.fc-dl-table td {
|
||||
text-align: left;
|
||||
padding: 0.2rem 0.5rem;
|
||||
border-bottom: 1px solid rgb(var(--v-theme-on-surface-variant) / 0.14);
|
||||
}
|
||||
.fc-dl-table th:last-child,
|
||||
.fc-dl-table td:last-child {
|
||||
text-align: right;
|
||||
font-variant-numeric: tabular-nums;
|
||||
}
|
||||
.fc-dl-table .fc-dl-empty {
|
||||
color: rgb(var(--v-theme-warning));
|
||||
}
|
||||
.fc-dl-blockhead {
|
||||
display: flex; align-items: center; justify-content: space-between;
|
||||
gap: 0.5rem;
|
||||
|
||||
@@ -60,11 +60,10 @@ def _make_fake_dl_result(
|
||||
*, success=True, written_paths=None, quarantined_paths=None,
|
||||
files_downloaded=0, error_type=None, error_message=None,
|
||||
stdout="", stderr="", cursor=None, run_stats=None, posts_processed=0,
|
||||
relink_source_paths=None, post_diagnostics=None,
|
||||
relink_source_paths=None,
|
||||
):
|
||||
return SimpleNamespace(
|
||||
relink_source_paths=relink_source_paths or [],
|
||||
post_diagnostics=post_diagnostics or [],
|
||||
success=success,
|
||||
url="https://patreon.com/alice",
|
||||
artist_slug="alice",
|
||||
@@ -821,28 +820,6 @@ async def test_recapture_mode_selected_and_flag_cleared_on_complete(
|
||||
assert "_backfill_recapture" not in co
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_diagnostics_written_to_event_metadata(
|
||||
db, db_sync, tmp_path, seed_artist_and_source,
|
||||
):
|
||||
"""#842: per-post body diagnostics land on the DownloadEvent.metadata_ so the
|
||||
UI can show how each post was handled (post_type + body length)."""
|
||||
_artist, source = seed_artist_and_source
|
||||
diag = [
|
||||
{"post_id": "p1", "title": "Has body", "post_type": "image_file", "body_chars": 42},
|
||||
{"post_id": "p2", "title": "Empty poll", "post_type": "poll", "body_chars": 0},
|
||||
]
|
||||
svc, _ = _backfill_svc(db, db_sync, tmp_path, _make_fake_dl_result(
|
||||
success=True, post_diagnostics=diag,
|
||||
))
|
||||
await svc.download_source(source.id)
|
||||
|
||||
ev = (await db.execute(
|
||||
select(DownloadEvent).order_by(DownloadEvent.id.desc()).limit(1)
|
||||
)).scalar_one()
|
||||
assert ev.metadata_["post_diagnostics"] == diag
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_partial_error_type_maps_to_ok_status(
|
||||
db, db_sync, tmp_path, seed_artist_and_source,
|
||||
|
||||
@@ -18,6 +18,7 @@ from backend.app.services.patreon_client import MediaItem
|
||||
from backend.app.services.patreon_downloader import (
|
||||
MediaOutcome,
|
||||
PatreonDownloader,
|
||||
PostRecordOutcome,
|
||||
_sanitize,
|
||||
)
|
||||
from backend.app.utils.sidecar import find_sidecar
|
||||
@@ -646,14 +647,17 @@ def test_write_post_record_writes_enriched_post_only_sidecar(tmp_path):
|
||||
)
|
||||
post = _post()
|
||||
post["attributes"]["content"] = "" # media-less post, empty feed body
|
||||
sc = dl.write_post_record(post, "artist-x")
|
||||
rec = dl.write_post_record(post, "artist-x")
|
||||
|
||||
assert sc is not None
|
||||
assert sc.name == "_post.json" # can't collide with a media `NN_*.json`
|
||||
data = json.loads(sc.read_text())
|
||||
assert isinstance(rec, PostRecordOutcome)
|
||||
assert rec.path is not None
|
||||
assert rec.path.name == "_post.json" # can't collide with a media `NN_*.json`
|
||||
data = json.loads(rec.path.read_text())
|
||||
assert data["id"] == "1001"
|
||||
assert data["content"] == "<p>text post body</p>"
|
||||
assert calls == ["1001"]
|
||||
# Outcome reports the captured body's shape (detail-fetched here).
|
||||
assert rec.body_chars == len("<p>text post body</p>")
|
||||
|
||||
|
||||
def test_write_post_record_none_without_post_id(tmp_path):
|
||||
@@ -661,4 +665,6 @@ def test_write_post_record_none_without_post_id(tmp_path):
|
||||
images_root=tmp_path, cookies_path=None, validate=False,
|
||||
session=_FakeSession(),
|
||||
)
|
||||
assert dl.write_post_record({"id": "", "attributes": {}}, "artist-x") is None
|
||||
rec = dl.write_post_record({"id": "", "attributes": {}}, "artist-x")
|
||||
assert rec.path is None
|
||||
assert rec.body_chars == 0
|
||||
|
||||
@@ -24,7 +24,7 @@ from backend.app.services.patreon_client import (
|
||||
PatreonAuthError,
|
||||
PatreonDriftError,
|
||||
)
|
||||
from backend.app.services.patreon_downloader import MediaOutcome
|
||||
from backend.app.services.patreon_downloader import MediaOutcome, PostRecordOutcome
|
||||
from backend.app.services.patreon_ingester import PatreonIngester, _ledger_key
|
||||
|
||||
pytestmark = pytest.mark.integration
|
||||
@@ -137,7 +137,14 @@ class _FakeDownloader:
|
||||
self.post_records += 1
|
||||
p = self.tmp_path / f"{post.get('id')}__post.json"
|
||||
p.write_text("{}")
|
||||
return p
|
||||
attrs = post.get("attributes") or {}
|
||||
body = attrs.get("content")
|
||||
return PostRecordOutcome(
|
||||
path=p,
|
||||
post_type=attrs.get("post_type"),
|
||||
title=attrs.get("title"),
|
||||
body_chars=len(body) if isinstance(body, str) else 0,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -654,13 +661,10 @@ async def test_backfill_skips_already_captured_post_but_recapture_forces_it(
|
||||
# Diagnostic summary surfaces the post-record + relink counts (operator reads
|
||||
# this off the event stdout to see what a recapture actually did).
|
||||
assert "1 post-record(s), 1 relinked" in res_recap.stdout
|
||||
# #842: per-post body diagnostic carries post_type + body length so the UI can
|
||||
# show how each post was handled (and explain empties by post_type).
|
||||
assert len(res_recap.post_diagnostics) == 1
|
||||
diag = res_recap.post_diagnostics[0]
|
||||
assert diag["post_id"] == "p1"
|
||||
assert diag["post_type"] == "image_file"
|
||||
assert diag["body_chars"] == len("<p>body p1</p>")
|
||||
# #842: per-post handling is written into the SAME run stdout (reuses the
|
||||
# existing "Raw stdout" panel) — post_type + body length, so an empty body is
|
||||
# self-explanatory by its post_type.
|
||||
assert f"post p1 [image_file] body: {len('<p>body p1</p>')} chars" in res_recap.stdout
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user