alkstore/tasks/pg-fix-dedupe-cleanup.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
pg-fix-dedupe-cleanup	Decode dedupe + small cleanups in queue/tx/notify (review 002 minor notes)	completed	pg-fix-tx-wake	narrow	low	component	implementation	wave-4-fixes postgres-engine cleanup

Description

Batch the wave-4 general review's minor notes that share files and are too small for standalone tasks (docs/reviews/002-wave-4-general-review.md §Minor notes + §Finding 6's queue bullet):

• job_from_row is byte-for-byte duplicated (queue.rs:411 and tx.rs:626; only pub(crate) differs), and get_job_tx inlines the 18-column dead/live select column lists (tx.rs:420-428, 438-442) duplicating queue.rs's live_columns()/dead_columns(). Both files claim "one decode owner" in their doc comments; both are wrong. The Job shape is contract-pinned, so two owners is exactly where a shape edit goes wrong later. Dedupe into one module — queue.rs is the natural home; tx.rs imports it (as tx.rs already imports stream_events_from_rows from stream.rs). The doc comments must again tell the truth (one decode owner).
• queue.rs:71 doc fix (Finding 6): sweep_expired "returns the moved count" — it returns moved + retention-deleted (queue.rs:720). The SQLite twin returns the same sum (substrate queue_ops.rs:232-243), so the engines agree; only the doc is wrong. ("Single-statement atomic" is also inaccurate — it's the in_tx multi-statement frame.) Correct both claims.
• notify.rs:106-110: the closed-store error is built inline (Error::database(std::io::Error::other(...))) instead of the shared database_error helper — one duplicate of the one message. Route it through the helper.
• bridge_capacity() (notify.rs:137-139) is a function returning a constant — a const says it plainly. Convert (keep the doc comment's rationale on the const).

No behavior changes anywhere in this task — it is dedupe + doc truth-telling. All existing tests must pass unchanged; where a test asserts on the moved-count semantics, it already pins the sum (the doc was the only liar).

Depends on pg-fix-tx-wake: that task edits tx.rs's publish/enqueue paths; this task restructures tx.rs's decode side — landing after avoids touching the same file regions blind.

Acceptance Criteria

• job_from_row has one owner (queue.rs); tx.rs imports it; both files' "one decode owner" doc claims are true
• get_job_tx's dead/live column lists reuse queue.rs's live_columns()/dead_columns() (no inlined duplicates)
• sweep_expired's doc states the moved + retention-deleted sum and the in_tx frame (matching the SQLite twin's semantics)
• notify.rs's closed-store error routes through database_error; bridge_capacity is a const
• Zero behavior change: the full pg lib suite green against the harness server, unchanged assertions
• cargo clippy --all-targets -- -D warnings, fmt clean; gates green server-less

References

• docs/reviews/002-wave-4-general-review.md §Minor notes + §Finding 6 (the queue bullet)
• docs/architecture/core-contract.md (the pinned Job shape — why one decode owner matters)
• tasks/pg-engine-queues.md, tasks/pg-engine-seam-tx.md (the tasks of record for the touched files)

Notes

• The dedupe landed as the task pinned: job_from_row is now owned by queue.rs (pub(crate)), tx.rs imports it; queue.rs's live_columns()/dead_columns() went pub(crate) so get_job_tx reuses them (the format!-bound {dead}/{live} select strings are byte-identical to the inlined lists, so zero SQL change). Tx-side imports ride the existing crate::queue module edge — fine crate-internally (queue.rs already imports tx.rs's encode_payload_bytes/enqueue_row; Rust module cycles are legal within a crate).
• Doc truth-telling choices: queue.rs's job_from_row doc now states the one owner explicitly ("the tx reads (crate::tx) ... import it"); tx.rs's trailing decode-owner comment now names both decode owners (stream decode in stream.rs, job decode in queue.rs) under ADR-012 §2; the removed tx.rs duplicate carried JobState's only remaining use, so that import was dropped. queue.rs's sweep_expired module-doc bullet now states the moved + retention-deleted sum and the in_tx multi-statement frame (matching the SQLite twin's semantics — the code already pinned the sum).
• bridge_capacity() → const BRIDGE_CAPACITY with the rationale doc comment carried; the closed-store error in listen now routes through database_error (it was the only inline duplicate; import already present).
• No behavior change anywhere: the select SQL strings, error shapes, and the bridge capacity value are identical.

Summary

Deduped the pg engine's job-row decode and cleaned the wave-4 minor notes (review 002): job_from_row has one owner (queue.rs), get_job_tx reuses live_columns()/dead_columns() instead of inlining the 18-column lists, sweep_expired's doc states the moved + retention-deleted sum in the in_tx frame (the sum the SQLite twin returns), notify.rs's closed-store error routes through database_error, and bridge_capacity is a const. Verified: pg suite 140/140 green against the harness server (pglo-poc :15432) and 140/140 server-less, unchanged assertions; workspace cargo build / cargo clippy --all-targets -- -D warnings / cargo fmt --check / cargo test --workspace all clean.