Skip to content

refactor: P29+P30 — dedup fake publish paths + loop-mode NOTIFY simulation#70

Merged
lesnik512 merged 1 commit into
mainfrom
worktree-p29-p30-test-broker-cleanup
Jun 13, 2026
Merged

refactor: P29+P30 — dedup fake publish paths + loop-mode NOTIFY simulation#70
lesnik512 merged 1 commit into
mainfrom
worktree-p29-p30-test-broker-cleanup

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Summary

The deferred test-broker cleanup, done together because they touch the same code.

P29 — dedup. The four fake publish paths — FakeOutboxProducer.publish / .publish_batch and the broker.publish / broker.publish_batch patches — each duplicated the same encode → feed → emit published → dispatch logic (~80 lines). Extracted two shared helpers, _fake_publish_one / _fake_publish_many (plus _emit_published); the four callers now resolve next_at and delegate. No behavior change — single-sourced logic. (The S5 batch ordering and the empty-batch guard are preserved in the helpers.)

P30 — loop-mode NOTIFY. In loop mode, a feed/publish now wakes the matching subscriber's _notify_event (new _notify_subscriber), mirroring production's pg_notify. Loop-mode tests no longer need a tight min_fetch_interval to be responsive. Wired into the shared helpers and TestOutboxBroker.feed.

Why they're together

P30's natural home is "after a row is inserted in the publish path" — exactly the duplicated code P29 consolidates. Adding the notify to all the feed sites without P29 would have deepened the duplication; with P29 it's one call in each shared helper.

Tests

Three loop-mode tests (via feed, broker.publish, broker.publish_batch) assert the handler runs within 2s under a deliberately slow 5s poll — only possible if the NOTIFY wakeup fired. Mutation-verified: disabling _notify_subscriber makes them time out.

Test Plan

  • Full dockerized suite: 475 passed, 100% coverage — the whole suite depends on the test broker, so the green run is the P29 no-behavior-change proof
  • ruff + ty clean
  • P30 mutation-verified (TimeoutError when notify disabled)

Net testing.py lines are ~flat (+9): the new shared helpers + P30 wiring offset the removed duplication, but the publish logic now lives in one place. Test-only — no production code touched.

This clears the last two deferred items from the 2026-06-12 audit (P27 is #69).

🤖 Generated with Claude Code

…ation

P29: the four fake publish paths (FakeOutboxProducer.publish/publish_batch and
the broker.publish/publish_batch patches) duplicated the same
"encode → feed → emit published → dispatch" logic. Extract two shared helpers
(_fake_publish_one / _fake_publish_many) plus _emit_published; the four callers
now just resolve next_at and delegate. Behavior is unchanged — validated by the
full test-broker-dependent suite (475 passed). Net lines are ~flat because the
helpers + P30 wiring offset the removed duplication, but the logic is now
single-sourced.

P30: in loop mode, a feed/publish now wakes the matching subscriber's
_notify_event (via the new _notify_subscriber), mirroring production NOTIFY.
Loop-mode tests no longer need a tight min_fetch_interval to be responsive.
Added to the shared helpers and to TestOutboxBroker.feed.

Tests: three loop-mode tests (feed / publish / publish_batch) assert the handler
runs within 2s under a deliberately slow 5s poll — only possible with the notify
wakeup (mutation-verified: TimeoutError when the notify is disabled).

Full dockerized suite: 475 passed, 100% coverage; ruff + ty clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lesnik512
lesnik512 merged commit 2b92459 into main Jun 13, 2026
3 checks passed
@lesnik512
lesnik512 deleted the worktree-p29-p30-test-broker-cleanup branch June 13, 2026 10:18
lesnik512 added a commit that referenced this pull request Jun 13, 2026
Covers the post-0.9.0 work: suspected findings S1–S5 (#68), the P27 warning-
attribution fix (#69), and the P29/P30 test-broker cleanup (#70). Patch bump —
fixes/hardening only, no new breaking changes; the one opt-in behavior change is
validate_schema() now catching partial-index predicate drift.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lesnik512 added a commit that referenced this pull request Jun 13, 2026
…ings (#71)

The 2026-06-12 code audit shipped across 0.9.0 (#61/#66/#67) and
0.9.1 (#68/#69/#70), but the invariant docs still described pre-fix
behavior. Sync the drift the audit flagged plus the drift its own
fixes introduced, and retire the now-resolved findings doc.

CLAUDE.md / architecture:
- relay: `_OutboxConfigError` no longer "rides AcknowledgementMiddleware
  to the nack path" — the worker catches it, logs at ERROR, and the row
  retries via lease expiry, not the retry_strategy (P18). Fixed in both
  CLAUDE.md and architecture/relay.md.
- terminal-write API: `delete_with_lease(conn, …)` /
  `mark_pending_with_lease(conn, …)`, not the `*_with_conn` names.
- NOTIFY: over-long table names now raise ValueError at construction
  (P7), not "silently lose NOTIFY and degrade to polling".
- engine ownership: drop the phantom `EngineState` holder and the dead
  `OutboxBrokerConfig.disconnect()` (removed in P24).
- schema: note the P8 `<table>_lease_ck` CHECK; validate_schema now
  also probes partial-index predicates (S2).
- drain regression-test references point at the real test names (T8
  added them to test_fake.py; integration covers the Postgres path).
- timers NOTIFY-skip: "genuinely future-dated" wording (a past/zero
  activate_at/in does fire NOTIFY).

Archive:
- move planning/active/2026-06-12-code-audit-findings.md →
  planning/archived/ with status: shipped + PR/release provenance.
- repoint the two release-notes "See also" links to archived/.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant