fix(meta): name the branch partition on every metadata statement

.claude/memory/reference_roborev_severity_threshold_met_is_clean.md

@@ -5,7 +5,7 @@ metadata:
   node_type: memory
   type: reference
   originSessionId: 05a4b981-1ec9-4115-91f7-2cdcd554c7ee
-  modified: 2026-07-29T17:33:37.873Z
+  modified: 2026-07-31T00:37:19.351Z
 ---
 
 When `roborev show <job>` prints only `SEVERITY_THRESHOLD_MET` as the review body, the commit **passed**. The string reads like "the severity threshold was met, so there are findings" — it means the opposite: the reviewer produced findings that all fell *below* `review_min_severity`, which `.roborev.toml` sets to `'medium'`, so they were suppressed and never surfaced.
@@ -14,4 +14,6 @@ The authoritative field is `roborev show <job> --json | jq .verdict_bool` — **
 
 So: an empty `kata list --label cha-NNN` alongside a `SEVERITY_THRESHOLD_MET` job is consistent, not a dropped finding. Don't go hunting for the suppressed text — check `verdict_bool` and move on.
 
+**Timing gotcha when polling for the verdict.** For a window after a job finishes, `roborev show <job> --json` exits non-zero with `Error: no review found for job <job>` while the plain `roborev show <job>` already renders the body — the review row lands after the job row. So a wait loop shaped like `until [ "$(roborev show N --json | jq -r '.status // "running"')" != "running" ]` **terminates immediately on that error**: jq gets no input, prints the empty string, and `"" != "running"` is true, which reads as "the job finished" when nothing was actually observed. Poll plain `roborev show` / `roborev list` for completion, then read `--json .verdict_bool` once. See [[feedback_absence_is_not_evidence]].
+
 Related: [[feedback_poll_roborev_after_any_commits]], and [[reference_roborev_kata_bridge_needs_cha_branch]] for the other direction — an empty queue is *not* evidence of a clean review when the bridge never ran at all.

crates/penca-api/src/lifecycle/compact.rs

@@ -357,12 +357,10 @@ where
     LifecycleManager::commit_compact_segment(&tx, &catalog_str, &branch_str, &merged_uri).await?;
 
     // Delete-set LAST, per the ordering invariant on
-    // `insert_segment_delete_set_rows`. This tx already holds a lock on the
-    // segment-metadata parent — its very first statement is
-    // `enumerate_unsealed_segments`, a `SELECT ... FOR UPDATE OF seg` against the
-    // catalog-wide parent — and the defer set is derived from that read, so
-    // compact cannot take a delete-set row lock before a parent lock even in
-    // principle. That is what fixes the global order for every other writer.
+    // `insert_segment_delete_set_rows`. Since CHA-546 every statement above
+    // names this branch's partitions, so the only parent lock this tx holds is
+    // the `AccessShare` its leaf INSERTs imply — compatible with the sweep's,
+    // so the invariant costs this path nothing.
     //
     // Still inside `tx`, which is what ADR 0019 §"Four-part mechanism" item 3
     // requires: the row must commit atomically with the URI swap.

crates/penca-api/src/lifecycle/retire.rs

@@ -178,11 +178,12 @@ impl LifecycleManager {
         )
         .await?;
 
-        // Delete-set LAST, after every segment-metadata parent this tx touches,
-        // per the ordering invariant on `insert_segment_delete_set_rows`. The
-        // sidecar URIs are read above (before their rows are deleted) but
-        // enqueued here, so the parent locks are all taken before any delete-set
-        // row lock. Position within the tx is free for ADR 0019 item 3 — it
+        // Delete-set LAST, per the ordering invariant on
+        // `insert_segment_delete_set_rows`. Since CHA-546 every statement above
+        // names this branch's partitions, and they are all SELECTs and DELETEs
+        // — which on a leaf take nothing on the parent — so this tx holds no
+        // segment-metadata parent lock at all and the invariant costs it
+        // nothing. Position within the tx is free for ADR 0019 item 3 — it
         // requires the rows to commit atomically with the retirement, not to
         // precede it.
         penca_storage_meta::LifecycleManager::insert_segment_delete_set_rows(

crates/penca-api/src/query/meta_plan.rs

@@ -369,7 +369,8 @@ impl QueryManager {
         w_snap: i64,
     ) -> Result<(i64, bool)> {
         let catalog = parse_uuid(catalog_uuid);
-        let purge_table = naming::table_purge_metadata_table(&catalog);
+        let branch = parse_uuid(branch_uuid);
+        let purge_table = naming::table_purge_metadata_partition(&catalog, &branch);
         let sql = format!(
             "WITH fence AS (\
                  SELECT GREATEST(\
@@ -389,7 +390,7 @@ impl QueryManager {
             delete = qi(delete_table_name),
         );
         let params = vec![
-            SqlValue::uuid_str(branch_uuid)?,
+            SqlValue::Uuid(branch),
             SqlValue::uuid_str(table_uuid)?,
             SqlValue::Int64(w_snap),
         ];
@@ -435,9 +436,10 @@ impl QueryManager {
         Option<BranchLineage>,
     )> {
         let catalog = parse_uuid(catalog_uuid);
+        let branch = parse_uuid(branch_uuid);
         let table = parse_meta_uuid(table_uuid, "table_uuid")?;
-        let persist_name = naming::table_persist_metadata_table(&catalog);
-        let snap_name = naming::table_snapshot_metadata_table(&catalog);
+        let persist_name = naming::table_persist_metadata_partition(&catalog, &branch);
+        let snap_name = naming::table_snapshot_metadata_partition(&catalog, &branch);
         // Fold the child's fork lineage into this already-per-read query (a
         // branch_store PK join) so a non-forked read pays no extra round-trip
         // for the base-source gate.
@@ -449,7 +451,7 @@ impl QueryManager {
         };
         let mut params: Vec<SqlValue> = vec![
             SqlValue::Uuid(table),
-            SqlValue::uuid_str(branch_uuid)?,
+            SqlValue::Uuid(branch),
             SqlValue::Int64(as_of_micros),
         ];
         if let Some(seq) = commit_seq_upper {
@@ -705,22 +707,22 @@ impl QueryManager {
         pinned_snapshot_uuid: Option<Uuid>,
     ) -> Result<SnapshotResult> {
         let catalog = parse_uuid(catalog_uuid);
+        let branch = parse_uuid(branch_uuid);
         // Parse fallibly (unlike the panicking `parse_uuid` above): a malformed
         // `table_uuid` surfaces as the same typed protocol error the
         // `meta_resolve` getters produce.
         let table = parse_meta_uuid(table_uuid, "table_uuid")?;
-        let snap_name = naming::table_snapshot_metadata_table(&catalog);
-        let seg_name = naming::table_snapshot_segment_metadata_table(&catalog);
+        let snap_name = naming::table_snapshot_metadata_partition(&catalog, &branch);
+        let seg_name = naming::table_snapshot_segment_metadata_partition(&catalog, &branch);
         // The internal row_uuid index parent/child, joined in below so a
         // planned snapshot segment carries its sidecar inline.
-        let idx_parent = naming::table_snapshot_index_metadata_table(&catalog);
-        let idx_child = naming::table_snapshot_segment_index_metadata_table(&catalog);
+        let idx_parent = naming::table_snapshot_index_metadata_partition(&catalog, &branch);
+        let idx_child = naming::table_snapshot_segment_index_metadata_partition(&catalog, &branch);
         // Params `$1` = table, `$2` = branch (fixed in the JOINs / WHERE); the
         // rest are bound in push order below, each `$N` computed from
         // `params.len()`, so the pinned-uuid and as_of/seq picks share one
         // numbering scheme.
-        let mut params: Vec<SqlValue> =
-            vec![SqlValue::Uuid(table), SqlValue::uuid_str(branch_uuid)?];
+        let mut params: Vec<SqlValue> = vec![SqlValue::Uuid(table), SqlValue::Uuid(branch)];
         let snapshot_selection = if let Some(uuid) = pinned_snapshot_uuid {
             params.push(SqlValue::Uuid(uuid));
             format!("snap.table_snapshot_uuid = ${}", params.len())
@@ -1027,7 +1029,8 @@ impl QueryManager {
         fork_commit_seq_num: i64,
     ) -> Result<Option<i64>> {
         let catalog = parse_uuid(catalog_uuid);
-        let seg = naming::table_persist_segment_metadata_table(&catalog);
+        let branch = parse_uuid(branch_uuid);
+        let seg = naming::table_persist_segment_metadata_partition(&catalog, &branch);
         let rows = driver
             .execute_params(
                 &format!(
@@ -1038,7 +1041,7 @@ impl QueryManager {
                     seg = qi(&seg),
                 ),
                 &[
-                    SqlValue::uuid_str(branch_uuid)?,
+                    SqlValue::Uuid(branch),
                     SqlValue::uuid_str(table_uuid)?,
                     SqlValue::Int64(fork_commit_seq_num),
                 ],
@@ -1248,8 +1251,15 @@ impl QueryManager {
         //   `commit_seq_num` already exceeds the cutoff. Composes with the
         //   committed_at tier fence; absent for the micros / OpenTx axes
         //   (`commit_seq_upper = None`).
-        let seg_table = naming::table_persist_segment_metadata_table(catalog_uuid);
-        let tfm_table = naming::table_persist_metadata_table(catalog_uuid);
+        //
+        // Both relations are named as the `branch_uuid` ARGUMENT's partitions,
+        // never an ambient current-branch value: `enumerate_base_cold_source`
+        // calls this with a fork's PARENT branch to resolve inherited cold,
+        // while the ordinary read path calls it with the reading branch. Taking
+        // the name from anywhere else compiles and silently reads the wrong
+        // branch's segments.
+        let seg_table = naming::table_persist_segment_metadata_partition(catalog_uuid, branch_uuid);
+        let tfm_table = naming::table_persist_metadata_partition(catalog_uuid, branch_uuid);
         let mut log_sql = format!(
             "SELECT tfm.log_kind, \
                     seg.table_persist_segment_uuid AS segment_uuid, \
@@ -1370,8 +1380,8 @@ impl QueryManager {
         let catalog = parse_uuid(catalog_uuid);
         let branch = parse_uuid(branch_uuid);
         let table = parse_uuid(table_uuid);
-        let seg_table = naming::table_persist_segment_metadata_table(&catalog);
-        let tfm_table = naming::table_persist_metadata_table(&catalog);
+        let seg_table = naming::table_persist_segment_metadata_partition(&catalog, &branch);
+        let tfm_table = naming::table_persist_metadata_partition(&catalog, &branch);
         let mut sql = format!(
             "SELECT MAX(seg.max_commit_seq_num) AS max_seq \
              FROM {seg} seg \

crates/penca-api/src/write/mod.rs

@@ -1086,28 +1086,38 @@ impl WriteManager {
         // safe — what the caller needs is to TELL a lock loss from a real failure,
         // which is why this maps to `Aborted` rather than the default `Internal`.
         //
-        // One conflict ordering cannot remove, TODO(CHA-546): a lifecycle write
-        // names the catalog-wide PARENT, so it holds ROW EXCLUSIVE there while
-        // waiting on this branch's leaf, and the drops below need ACCESS
-        // EXCLUSIVE on that same parent — a cycle, measured, which Postgres
-        // resolves by killing teardown. Rare (it needs a metadata write on this
-        // branch inside teardown's window) and clean (full rollback, reported as
-        // `Aborted`, succeeds on reissue). CHA-546 converts those writes to name
-        // the partition, as the tx-log family already does, which removes it.
+        // One conflict ordering cannot remove: the drops below need ACCESS
+        // EXCLUSIVE on the catalog-wide parents, which conflicts with EVERY
+        // mode, so any concurrent holder of any mode on a parent delays them.
+        // Two such holders survive CHA-546 by design. CHA-531's refcount gate
+        // reads those parents catalog-wide, and — measured, not assumed — an
+        // INSERT or UPDATE naming a LEAF still takes `AccessShare` on its
+        // parent to evaluate the partition constraint, held to commit. So any
+        // in-flight writer on any branch in the catalog delays these drops.
+        //
+        // Both are waits, not cycles — neither holder takes a lock this
+        // transaction holds — so they surface as a `lock_timeout` rather than a
+        // deadlock kill, and are clean either way (full rollback, reported as
+        // `Aborted`, succeeds on reissue). What CHA-546 bought here is the
+        // EXCLUSIVE step above rather than these drops: `AccessShare` clears
+        // it, the pre-CHA-546 `RowExclusive` did not.
         //
         // The branch's HOT data tables (`schema_uuid = None` = catalog-wide),
         // resolved BEFORE the teardown transaction and used only for their drops.
         //
-        // It cannot move inside. `list_table_uuids_for_branch` plans through
-        // `QueryManager::plan`, which reads the catalog-wide metadata parents BY
-        // NAME — so running it in the transaction takes `ACCESS SHARE` on every
-        // one of them, which is precisely what
-        // `lock_branch_teardown_partitions` is built to avoid: it stalls plans on
-        // every branch in the catalog while this cold-capable read waits on
-        // object storage, and it sets up the `ACCESS SHARE` -> `ACCESS EXCLUSIVE`
-        // upgrade at the drops that deadlocks two concurrent teardowns of
-        // DIFFERENT branches. Partition-scoping the enumerations bought exactly
-        // that property; planning inside the lock gives it back.
+        // It cannot move inside, and since CHA-546 the reason is duration
+        // rather than lock footprint. `list_table_uuids_for_branch` resolves
+        // `sys_tables` through the full read path, which names only this
+        // branch's partitions — so it would take no parent lock. What it does
+        // take is unbounded time: it is a cold-capable read that waits on
+        // object storage, and NOTHING bounds it. `lock_timeout` governs lock
+        // acquisition, not execution, and no `statement_timeout` is set on
+        // this path — so inside the transaction that wait runs to completion
+        // while this branch's 14 leaves are held EXCLUSIVE, blocking every
+        // writer on the branch for the length of an S3 fetch. It also widens
+        // the window before the drops, which must then win ACCESS EXCLUSIVE on
+        // the catalog-wide parents within the 5s bound — the one place that
+        // bound does bite (see above).
         //
         // The cost is a real leak, stated plainly rather than filed under
         // "best effort": a `CreateTable` committing between this read and the
@@ -1117,8 +1127,8 @@ impl WriteManager {
         // delete-set consequence — but they are permanent.
         //
         // Pre-existing, not introduced here: `main` resolves this list the same
-        // way. Closing it needs a branch-scoped table enumeration that does not
-        // plan through the parents, which does not exist today — the hot data
+        // way. Closing it needs a branch-scoped table enumeration that cannot
+        // reach object storage, which does not exist today — the hot data
         // relations are named `hash(table_uuid, branch_uuid)`, so they cannot be
         // recovered from `pg_class` by branch either. Needs its own ticket.
         let table_uuid_strs = self
@@ -1241,12 +1251,13 @@ impl WriteManager {
                 LifecycleManager::drop_branch_partitions(tx, catalog_str, branch_str).await?;
 
                 // Delete-set LAST, after the partition drops, per the ordering
-                // invariant on `insert_segment_delete_set_rows`. Dropping a
-                // partition takes ACCESS EXCLUSIVE on the catalog-wide parent; a
-                // concurrent compact holds ROW SHARE on that same parent from its
-                // opening `SELECT ... FOR UPDATE` and cannot release it, so
-                // teardown must not be holding a delete-set row while it waits
-                // for the parent. Still one transaction, so removing the
+                // invariant on `insert_segment_delete_set_rows`. Teardown is the
+                // one writer still subject to it: dropping a partition takes
+                // ACCESS EXCLUSIVE on the catalog-wide parent, and the sweep's
+                // refcount gate holds AccessShare on that same parent — CHA-531
+                // keeps those probes catalog-wide — while it locks delete-set
+                // rows. So teardown must not be holding a delete-set row while it
+                // waits for the parent. Still one transaction, so removing the
                 // references and queueing the files remain one atomic fact.
                 LifecycleManager::insert_segment_delete_set_rows(tx, catalog_str, &queued_uris)
                     .await?;

crates/penca-core/src/naming/tables.rs

@@ -551,4 +551,17 @@ mod tests {
             "6830ca7e-5210-6616-91cf-34c0e0d7c612_tx_table_log_partition"
         );
     }
+
+    #[test]
+    fn test_parity_table_snapshot_segment_metadata_partition() {
+        // CHA-546: the first METADATA leaf a Python caller has to name.
+        // Integration tests count PG statements by relation name, so the
+        // client must compute byte-identical leaf names to the server —
+        // a mismatch makes a `== 0` statement-count assertion pass
+        // vacuously rather than fail. Golden mirrors the Python suite.
+        assert_eq!(
+            table_snapshot_segment_metadata_partition(&CAT, &BR),
+            "1a93a047-8229-c30f-2de8-08a4482c2051_table_snapshot_segment_metadata_partition"
+        );
+    }
 }

crates/penca-db/src/dialect/pg.rs

@@ -1380,21 +1380,26 @@ impl PgDialect {
             .join(", ");
         // `SET LOCAL` is TRANSACTION-scoped, not statement-scoped, so this bound
         // governs every later statement too — including the 14 `DROP TABLE`s,
-        // which is where it actually bites. That is deliberate but worth stating,
-        // because the dominant conflict is not another teardown: `compact` opens
-        // with `SELECT ... FOR UPDATE OF seg` naming the catalog-wide
-        // `table_persist_segment_metadata` PARENT, so it holds `ROW SHARE` there
-        // across its whole cold read and merged write. A drop needs `ACCESS
-        // EXCLUSIVE` on that parent, so a compact running anywhere in the catalog
-        // for longer than this bound fails the teardown.
+        // which is where it actually bites, since `ACCESS EXCLUSIVE` on a parent
+        // conflicts with every mode while the `EXCLUSIVE` taken here does not
+        // conflict with `AccessShare`.
+        //
+        // On this branch, its own lifecycle work holds `ROW SHARE` /
+        // `ROW EXCLUSIVE` on the very leaves locked here — deleting a branch
+        // while writing it is the caller's own race, and the `EXCLUSIVE` below
+        // is what catches it. Catalog-wide, two holders can still delay the
+        // drops: CHA-531's refcount gate, which probes the segment-metadata
+        // parents in a single statement, and any in-flight writer on any branch,
+        // because an `INSERT`/`UPDATE` naming a LEAF still takes `AccessShare`
+        // on its parent to evaluate the partition constraint (measured — reads
+        // and `DELETE`s take nothing). Both are bounded by a statement or a
+        // transaction rather than by a whole lifecycle wave, which is the
+        // difference CHA-546 made.
         //
         // Failing is the right end of the trade — waiting instead would queue
         // every subsequent lock request on that parent behind us, turning one
-        // slow compact into a catalog-wide stall. The caller reports it as
-        // `Aborted` and reissues. TODO(CHA-546): the root cause is compact naming
-        // the parent rather than the branch's partition; once reads and writes
-        // both target partitions, teardown's drops contend with nothing outside
-        // the branch and this stops being reachable in steady state.
+        // slow writer into a catalog-wide stall. The caller reports it as
+        // `Aborted` and reissues.
         //
         // Both errors propagate UNWRAPPED. Re-wrapping as `sqlx::Error::Protocol`
         // would erase the `Error::Database` variant, and `as_database_error()`