Commit 3bf90d0
authored
fix: tighten publish-event metric accuracy (#204)
* fix: tighten publish-event metric accuracy
Five accuracy fixes to the publish-event and search-indexing metric
subscribers so the recorded events match the SDD contract's "once per
post" / "one per network when the async result is known" semantics.
1. AP outbox: gate fosse_publish_result + the MC success bump to a post
Create. The activitypub_outbox_processing_complete hook carries no
activity-type filter, so it previously bumped for every dispatch
(Updates, Deletes, comment activities, actor-profile Updates, and a
second Announce per post in ACTIVITYPUB_ACTOR_AND_BLOG_MODE, which
double-counted). Discriminate via the outbox item's
_activitypub_activity_type (Create) plus _activitypub_object_id
resolving to a real post through url_to_postid(), which also separates
post Creates from comment Creates. In-memory and persisted aggregate
state is still cleared for filtered-out activities so no metric meta
leaks.
2. Bluesky strategy: seed atmosphere_is_short_form_post with Atmosphere's
own shape predicate (no title support OR empty title OR a post format)
instead of a hardcoded false, so titleless / post-format posts that
Atmosphere publishes short-form are no longer misrecorded as long-form.
3. Atmosphere publish result: skip both events on the not-publishable
early return (atmosphere_post_not_publishable), and suppress the
funnel-entry fosse_post_published during the Backfill AJAX action.
fosse_publish_result still fires for backfill since a real write
happened. Documented gap: the update-falls-through-to-publish retry is
indistinguishable from a first publish at this hook without probing
bundled-plugin private meta, so it is still counted.
4. Network spelling: align the fediverse publish event to network
'mastodon' and the MC bump to fosse-publish-success-mastodon, matching
the SDD taxonomy (connection funnel enum + MC bump list) which already
use mastodon/bluesky.
5. Search-indexing watcher: add a register() idempotence guard mirroring
Publish_Events, correct the docblock (update_option short-circuits
identical writes, so the action only fires on genuine changes), and
remove the 30s debounce whose stated rationale was invalid and which
risked dropping genuine rapid 1->0 flips.
* fix: suppress funnel entry for update/delete cron re-publishes
Review follow-up on the metric-accuracy pass. Two gaps:
1. The known-gap note claimed the only residual fosse_post_published
over-count was the retry of a previously-failed publish. It missed
Publisher::update_post() -> rewrite_thread() -> publish_post(), which
fires atmosphere_publish_post_result for every shape-changing edit of
an already-live post (and the atmosphere_delete_post publishable
reconcile does the same). Every publish_post() reached from inside the
atmosphere_update_post / atmosphere_delete_post cron callbacks is a
re-publish or an already-counted retry, never a first publish — the
pristine-post case early-returns via
atmosphere_update_skipped_unsynced_post before the hook. Suppress
fosse_post_published while either cron action runs, mirroring the
backfill technique; fosse_publish_result still fires since a real AT
Protocol write occurred. The residual gap narrows to the
unpublish-then-republish race through the atmosphere_publish_post cron.
2. resolve_strategy() coerced the atmosphere_is_short_form_post filter
result with a (bool) cast, but Atmosphere's is_short_form_post()
wrapper uses wp_validate_boolean() — a filter returning the string
'false' published long-form upstream while being recorded short-form.
Align the coercion.
Tests: republish-cron suppression (both actions), first-publish cron
still records the funnel entry, and 'false'-string filter parity. All
three suppression/parity tests fail against the unpatched handler.
* fix: allowlist the publish cron for the funnel-entry event
Upstream Atmosphere trunk replaced the AJAX backfill with a WP-CLI
command that calls Publisher::publish_post() directly, with no marker
action a deny-list could detect — re-syncs of historical posts would
flood fosse_post_published again. Flip the first-publish gate to an
allowlist on the atmosphere_publish_post cron action, the one context
that is always a genuine first publish in both bundle generations.
fosse_publish_result still fires from every context. The previous
deny-list constants collapse into the allowlist's rationale; existing
suppression tests keep passing, and a new test pins the action-less
CLI-backfill context.
* fix: idempotent AP publish-result emit per source post
The bundled scheduler emits another `Create` outbox item when a
previously deleted/federated post becomes publicly queryable again
(AP "resurrection" path). The previous gate only checked activity
type + object URL resolution, so the same WordPress post would tick
`fosse-publish-success-mastodon` and emit a second `fosse_publish_result`
on every resurrection — defeating the at-most-once metric goal.
Add a per-source-post idempotency marker
(`_fosse_ap_publish_recorded` postmeta) on the resolved source post.
Skip the emit when the marker is set; write the marker after a
successful emit. The marker lives on the source post (not the outbox
item) so it survives outbox cleanup, and is dropped on hard delete
along with the rest of the post's metadata — a fresh-ID republish
correctly counts as a new publish.
Regression test seeds a first Create + dispatch (emits + sets marker),
resets the recorder channels, then dispatches a second Create with
the same object URL and asserts neither event nor counter fire.
* fix: status-aware AP publish-result marker — failure → success allowed
The previous boolean marker was set after every emit, including
failures. If the first Create for a source post had zero successful
inbox sends, the marker latched on `1`, and a later same-source
Create whose inbox sends DID succeed (a retry, or AP's resurrection
path firing after the original attempt failed) would hit the early
return — `fosse_publish_result` never recorded, the
`fosse-publish-success-mastodon` counter never bumped. That makes
the success metric depend on the FIRST attempt's outcome rather than
whether the post ever successfully federated.
Persist the LAST recorded status instead of a bare boolean. Only
`'success'` is a no-emit terminal state; `'failure'` allows the
next emit so failure → success can record. Success → success stays
suppressed for the resurrection-dedup goal.
Regression test: first Create's inbox sends all fail, then a
later same-source Create succeeds. Asserts the success event +
counter bump record correctly.
* fix: dedup Bluesky funnel entry and classify custom-text posts
Review follow-ups (ce-code-review + codex) on the metrics-accuracy work:
- fosse_post_published double-counted on transient-failure retries: the
atmosphere_publish_post cron also wraps retries and the
has_post_records() retry-as-fresh-publish branch, so a fail-then-succeed
cycle recorded the funnel entry twice, breaking the once-per-post
contract. Add a per-post idempotency marker
(POST_PUBLISHED_RECORDED_META_KEY), mirroring the AP path's marker, so
the entry records at most once per post ID.
- Custom-text posts publish as a single link card upstream regardless of
shape (Atmosphere's is_short_form_post() returns false before the filter
when custom text is set). resolve_strategy() now short-circuits saved
custom text to link-card-fallback instead of misrecording titleless
custom-text posts as short-form-note.
- Fix 2 short-form seed: document it as a shape-only approximation (upstream
2.0.0 adds a >300-grapheme no-image long-form branch FOSSE deliberately
does not replicate) rather than overclaiming an exact predicate.
- Refresh the WP-CLI-backfill docblock/test wording for bundled 2.0.0, and
reword the first-publish gate as a heuristic backed by the marker.1 parent 1f78a16 commit 3bf90d0
4 files changed
Lines changed: 1088 additions & 90 deletions
File tree
- src/Metrics
- tests/php/Metrics
0 commit comments